Skip to content

MULTIARCH-5369: Implement logic in the pod reconciler to work with the namespace-scoped PPC #676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

MULTIARCH-5369: Implement logic in the pod reconciler to work with the namespace-scoped PPC #676

wants to merge 4 commits into from

Conversation

@AnnaZivkovic
Copy link
Member

@AnnaZivkovic AnnaZivkovic commented Aug 16, 2025 •
edited
Loading

Given a pod hitting the pod reconciliation loop:

  • Verify the pod has the scheduling gate (return if not)
  • Get all the pod placement configurations in the pod's namespace, sorted by descending priority
  • For each pod placement configuration:
    • Check if the pod matches the label selector
    • Apply the configuration to the pod.
    • If a proposed configuration overlaps with the current pod specification, it is not applied and an event is recorded in the pod's event log. This ensures that the operator does not overwrite the user's configuration and that a PodPlacementConfig CRD with a higher priority gets precedence over the ones with lower priority.
  • Apply the cluster-scoped ClusterPodPlacementConfig configuration to the pod. As the namespace-scoped configuration has already been applied, the cluster-scoped configuration will not overwrite any overlapping namespace-scoped configuration. We can also garantee that the cluster-scoped ClusterPodPlacementConfig's strong predicates will be applied to the pods as they are not handled by the namespace-scoped resource, which is focused on the scheduling scoring plugins.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 16, 2025
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 16, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Aug 16, 2025 •
edited by openshift-ci bot
Loading

@AnnaZivkovic: This pull request references MULTIARCH-5369 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.20." or "openshift-4.20.", but it targets "mto-1.2" instead.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci
Copy link

openshift-ci bot commented Aug 16, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AnnaZivkovic

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 16, 2025
@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch from ad983c3 to ce0027e Compare August 18, 2025 21:48
@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch from ce0027e to 26e1ba1 Compare September 16, 2025 19:18
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Sep 16, 2025 •
edited by openshift-ci bot
Loading

@AnnaZivkovic: This pull request references MULTIARCH-5369 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.21." or "openshift-4.21.", but it targets "mto-1.2" instead.

In response to this:

Given a pod hitting the pod reconciliation loop:

  • Verify the pod has the scheduling gate (return if not)
  • Get all the pod placement configurations in the pod's namespace, sorted by descending priority
  • For each pod placement configuration:
    • Check if the pod matches the label selector
    • Apply the configuration to the pod.
    • If a proposed configuration overlaps with the current pod specification, it is not applied and an event is recorded in the pod's event log. This ensures that the operator does not overwrite the user's configuration and that a PodPlacementConfig CRD with a higher priority gets precedence over the ones with lower priority.
  • Apply the cluster-scoped ClusterPodPlacementConfig configuration to the pod. As the namespace-scoped configuration has already been applied, the cluster-scoped configuration will not overwrite any overlapping namespace-scoped configuration. We can also garantee that the cluster-scoped ClusterPodPlacementConfig's strong predicates will be applied to the pods as they are not handled by the namespace-scoped resource, which is focused on the scheduling scoring plugins.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@AnnaZivkovic AnnaZivkovic changed the title WIP: MULTIARCH-5369: Implement logic in the pod reconciler to work with the namespace-scoped MULTIARCH-5369: Implement logic in the pod reconciler to work with the namespace-scoped Sep 16, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 16, 2025
@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch from 26e1ba1 to 7a09ecd Compare September 16, 2025 19:35
@AnnaZivkovic
Copy link
Member Author

AnnaZivkovic commented Sep 17, 2025

/retest

@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch 3 times, most recently from 8412eaf to 6ca8b43 Compare September 19, 2025 17:42
lwan-wanglin
lwan-wanglin reviewed Sep 24, 2025
View reviewed changes
controllers/podplacement/pod_model.go
// SetPreferredArchNodeAffinity sets the node affinity for the pod to the preferences given in the ClusterPodPlacementConfig.
func (pod *Pod) SetPreferredArchNodeAffinity(cppc *v1beta1.ClusterPodPlacementConfig) {
// Prevent overriding of user-provided kubernetes.io/arch preferred affinities or overwriting previously set preferred affinity
if pod.isPreferredAffinityConfiguredForArchitecture() {
Copy link
Member

@lwan-wanglin lwan-wanglin Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AnnaZivkovic, Why are we removing this check? Should we ignore Pods if the user has already set a Preferred Affinity for architecture?
If the intention is to cover both local PPC and CPPC, could we introduce a third option to distinguish whether the Preferred Affinity was user-defined or modified by our code? For example, we might rely on the label multiarch.openshift.io/preferred-node-affinity—treat Pods without this label(or not-set) or with the value set differently.

Copy link
Member Author

@AnnaZivkovic AnnaZivkovic Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lwan-wanglin that is a good idea! ill add a value to notate how the label got set.

This check will need to be removed or modified because it can not find the difference between having the preferred node affinity setp with the CPPC or the PPC. With the original check it would not check the CPPC preferred affinity if the PPC set anything. Tho I think removing it is the wrong move now. Ill give it a more fine grained approach

@AnnaZivkovic
Copy link
Member Author

AnnaZivkovic commented Oct 15, 2025

/test ci/prow/ocp416-ci-index-multiarch-tuning-operator-bundle

@openshift-ci
Copy link

openshift-ci bot commented Oct 15, 2025

@AnnaZivkovic: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test bundle

/test ci-index-multiarch-tuning-operator-bundle

/test fmt

/test generate

/test goimports

/test images

/test lint

/test manifests

/test ocp416-ci-index-multiarch-tuning-operator-bundle

/test ocp416-e2e-gcp

/test ocp416-images

/test ocp417-ci-index-multiarch-tuning-operator-bundle

/test ocp417-e2e-gcp

/test ocp417-images

/test ocp418-ci-index-multiarch-tuning-operator-bundle

/test ocp418-e2e-azure

/test ocp418-images

/test ocp419-ci-index-multiarch-tuning-operator-bundle

/test ocp419-e2e-azure

/test ocp419-images

/test ocp420-ci-index-multiarch-tuning-operator-bundle

/test ocp420-e2e-gcp

/test ocp420-images

/test sast

/test unit

/test vendor

/test vet

The following commands are available to trigger optional jobs:

/test ocp417-azure-heterogeneous-perfscale

/test ocp417-azure-mto-heterogeneous-perfscale

/test security

Use /test all to run the following jobs that were automatically triggered:

pull-ci-outrigger-project-multiarch-tuning-operator-main-bundle

pull-ci-outrigger-project-multiarch-tuning-operator-main-ci-index-multiarch-tuning-operator-bundle

pull-ci-outrigger-project-multiarch-tuning-operator-main-fmt

pull-ci-outrigger-project-multiarch-tuning-operator-main-generate

pull-ci-outrigger-project-multiarch-tuning-operator-main-goimports

pull-ci-outrigger-project-multiarch-tuning-operator-main-images

pull-ci-outrigger-project-multiarch-tuning-operator-main-lint

pull-ci-outrigger-project-multiarch-tuning-operator-main-manifests

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp416-ci-index-multiarch-tuning-operator-bundle

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp416-e2e-gcp

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp416-images

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp417-ci-index-multiarch-tuning-operator-bundle

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp417-e2e-gcp

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp417-images

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp418-ci-index-multiarch-tuning-operator-bundle

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp418-e2e-azure

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp418-images

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp419-ci-index-multiarch-tuning-operator-bundle

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp419-e2e-azure

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp419-images

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp420-ci-index-multiarch-tuning-operator-bundle

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp420-e2e-gcp

pull-ci-outrigger-project-multiarch-tuning-operator-main-ocp420-images

pull-ci-outrigger-project-multiarch-tuning-operator-main-sast

pull-ci-outrigger-project-multiarch-tuning-operator-main-security

pull-ci-outrigger-project-multiarch-tuning-operator-main-unit

pull-ci-outrigger-project-multiarch-tuning-operator-main-vendor

pull-ci-outrigger-project-multiarch-tuning-operator-main-vet

In response to this:

/test ci/prow/ocp416-ci-index-multiarch-tuning-operator-bundle

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@AnnaZivkovic
Copy link
Member Author

AnnaZivkovic commented Oct 15, 2025

/test ci-index-multiarch-tuning-operator-bundle

Testing unrelated to pr. Looking for infrastructure errors

@AnnaZivkovic
Copy link
Member Author

AnnaZivkovic commented Nov 6, 2025

/retest

@AnnaZivkovic
Copy link
Member Author

AnnaZivkovic commented Nov 12, 2025

/retest

@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch 3 times, most recently from b745c58 to 168894e Compare November 13, 2025 00:23
@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch from 168894e to 43b1f57 Compare November 14, 2025 00:16
@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch 3 times, most recently from acb5662 to b0da1c3 Compare November 14, 2025 18:55
@AnnaZivkovic AnnaZivkovic force-pushed the pod-reconciler-add-PodPlacementConfig branch from b0da1c3 to 0b86c94 Compare November 15, 2025 01:34
@AnnaZivkovic AnnaZivkovic changed the title MULTIARCH-5369: Implement logic in the pod reconciler to work with the namespace-scoped MULTIARCH-5369: Implement logic in the pod reconciler to work with the namespace-scoped PPC Nov 15, 2025
@AnnaZivkovic
Copy link
Member Author

AnnaZivkovic commented Nov 17, 2025

/retest

1 similar comment
@AnnaZivkovic
Copy link
Member Author

AnnaZivkovic commented Nov 18, 2025

/retest

@openshift-ci
Copy link

openshift-ci bot commented Nov 18, 2025

@AnnaZivkovic: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocp418-e2e-gcp 6ca8b43 link true /test ocp418-e2e-gcp
ci/prow/ocp419-e2e-gcp 6ca8b43 link true /test ocp419-e2e-gcp
ci/prow/ocp419-e2e-azure 0b86c94 link true /test ocp419-e2e-azure
ci/prow/ocp418-e2e-azure 0b86c94 link true /test ocp418-e2e-azure

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aleskandro aleskandro Awaiting requested review from aleskandro

1 more reviewer

@lwan-wanglin lwan-wanglin lwan-wanglin left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AnnaZivkovic @openshift-ci-robot @lwan-wanglin