Skip to content
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.

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

CNF-11559: [Part 3/3] Hypershift PAO adoption #1057

Merged
merged 14 commits into from
Jun 9, 2024

Conversation

Tal-or
Copy link
Contributor

@Tal-or Tal-or commented May 9, 2024

This PR provides the actual support of PAO/NTO over hypershift platform.
The PR follows the enhancement: openshift/enhancements#1244

More details in the commits themselves.

  • Implementation
  • E2E config suite adoption

Additional gaps which should be addressed on separate PRs, but listed here for transparency:

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 9, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 9, 2024

@Tal-or: This pull request references CNF-11559 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 the "4.16.0" version, but no target version was set.

In response to this:

This PR provides the actual support of PAO/NTO over hypershift platform.
The PR follows the enhancement: openshift/enhancements#1244

More details in the commits themselves.

  • Implementation
  • E2E testing

Additional gaps which should be addressed but on separate PRs:

  • Status reporting
  • Retrieve containerruntimeconfig information

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 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 May 9, 2024
@openshift-ci openshift-ci bot requested review from dagrayvid and yanirq May 9, 2024 14:11
@Tal-or
Copy link
Contributor Author

Tal-or commented May 12, 2024

/retest

@Tal-or Tal-or force-pushed the pao_adoption_phase3 branch from a72d1dd to 9195346 Compare May 15, 2024 08:23
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 15, 2024

@Tal-or: This pull request references CNF-11559 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 the "4.16.0" version, but no target version was set.

In response to this:

This PR provides the actual support of PAO/NTO over hypershift platform.
The PR follows the enhancement: openshift/enhancements#1244

More details in the commits themselves.

  • Implementation
  • E2E testing

Additional gaps which should be addressed but on separate PRs:

  • Status reporting
  • Retrieve containerruntimeconfig information
  • Feature Gate support

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-robot
Copy link
Contributor

openshift-ci-robot commented May 15, 2024

@Tal-or: This pull request references CNF-11559 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 the "4.16.0" version, but no target version was set.

In response to this:

This PR provides the actual support of PAO/NTO over hypershift platform.
The PR follows the enhancement: openshift/enhancements#1244

More details in the commits themselves.

  • Implementation
  • E2E testing

Additional gaps which should be addressed but on separate PRs:

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-robot
Copy link
Contributor

openshift-ci-robot commented May 15, 2024

@Tal-or: This pull request references CNF-11559 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 the "4.16.0" version, but no target version was set.

In response to this:

This PR provides the actual support of PAO/NTO over hypershift platform.
The PR follows the enhancement: openshift/enhancements#1244

More details in the commits themselves.

  • Implementation
  • E2E testing

Additional gaps which should be addressed on separate PRs, but listed here for transparency:

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.

@Tal-or Tal-or force-pushed the pao_adoption_phase3 branch from 9195346 to 5e01e76 Compare May 15, 2024 08:47
@Tal-or
Copy link
Contributor Author

Tal-or commented May 15, 2024

depends on: #1004

@Tal-or Tal-or force-pushed the pao_adoption_phase3 branch from 5e01e76 to 833c724 Compare May 16, 2024 09:22
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 16, 2024

@Tal-or: This pull request references CNF-11559 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 the "4.16.0" version, but no target version was set.

In response to this:

This PR provides the actual support of PAO/NTO over hypershift platform.
The PR follows the enhancement: openshift/enhancements#1244

More details in the commits themselves.

  • Implementation
  • E2E testing

Additional gaps which should be addressed on separate PRs, but listed here for transparency:

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.

@Tal-or
Copy link
Contributor Author

Tal-or commented May 22, 2024

/retest

@Tal-or Tal-or force-pushed the pao_adoption_phase3 branch from d150863 to 3849355 Compare May 22, 2024 13:17
Tal-or

This comment was marked as off-topic.

@Tal-or Tal-or force-pushed the pao_adoption_phase3 branch 2 times, most recently from 445e82f to 4c5cbe5 Compare May 28, 2024 06:29
@Tal-or
Copy link
Contributor Author

Tal-or commented May 28, 2024

/test e2e-hypershift-pao

@Tal-or
Copy link
Contributor Author

Tal-or commented May 28, 2024

/retest

@Tal-or Tal-or force-pushed the pao_adoption_phase3 branch from 4c5cbe5 to 8388f77 Compare May 29, 2024 11:58
@Tal-or Tal-or changed the title WIP: CNF-11559: [Part 3/3] Hypershift PAO adoption CNF-11559: [Part 3/3] Hypershift PAO adoption May 29, 2024
@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 May 29, 2024
Tal-or added 12 commits June 6, 2024 08:19
status reporting is not supported on hypershift yet.
we provide a fake implementation in order to conforms the interface.

Signed-off-by: Talor Itzhak <[email protected]>
Checking the feature gate first will result with an error on hypershift since it's not yet supported.

Signed-off-by: Talor Itzhak <[email protected]>
Adding a management client to be used on a hypershift platform.

Signed-off-by: Talor Itzhak <[email protected]>
Add the necessary configuration for the controller, so it would be able to run on a hypershift platform.

1. It needs to configure predicates for ConfigMaps instead of actual objects.
2. It needs to setup different clients for the management cluster and hosted cluster.

Signed-off-by: Talor Itzhak <[email protected]>
Modify the reconciliation flow so it could fit for both HCP and OCP.

Signed-off-by: Talor Itzhak <[email protected]>
Adding the relevant code for configuring performanceProfile over hypershift

Signed-off-by: Talor Itzhak <[email protected]>
Bump needed for handling hypershift APIs.
This is the output formatted and generated by the \`go mod tidy\`

Signed-off-by: Talor Itzhak <[email protected]>
The package that holds featuregate geatures has changed in the last vendor bump.
This commit uses the new paths.

Signed-off-by: Talor Itzhak <[email protected]>
- Move logs to V(4)
- Fix typos
- Remove redundant spacing
- Consistent HyperShift naming

Signed-off-by: Talor Itzhak <[email protected]>
@Tal-or Tal-or force-pushed the pao_adoption_phase3 branch from 7ef0009 to 2391f85 Compare June 6, 2024 05:21
@Tal-or
Copy link
Contributor Author

Tal-or commented Jun 6, 2024

Depends on openshift/release#52876

@jmencak
Copy link
Contributor

jmencak commented Jun 7, 2024

/retest

@Tal-or
Copy link
Contributor Author

Tal-or commented Jun 7, 2024

/test e2e-hypershift

Copy link
Contributor

@ffromani ffromani left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 7, 2024
@ffromani
Copy link
Contributor

ffromani commented Jun 7, 2024

/hold cancel

because missing approval so no risk of premature merge

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 7, 2024
@ffromani
Copy link
Contributor

ffromani commented Jun 7, 2024

/lgtm

@yanirq
Copy link
Contributor

yanirq commented Jun 9, 2024

/approve

Copy link
Contributor

openshift-ci bot commented Jun 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Tal-or, yanirq

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 Jun 9, 2024
Copy link
Contributor

openshift-ci bot commented Jun 9, 2024

@Tal-or: all tests passed!

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 4d6cb12 into openshift:master Jun 9, 2024
16 checks passed
@Tal-or Tal-or deleted the pao_adoption_phase3 branch June 9, 2024 14:05
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build cluster-node-tuning-operator-container-v4.17.0-202406101017.p0.g4d6cb12.assembly.stream.el9 for distgit cluster-node-tuning-operator.
All builds following this will include this PR.

rbaturov pushed a commit to rbaturov/cluster-node-tuning-operator that referenced this pull request Jun 10, 2024
* pao:hypershift: do not add mco scheme

On hypershift we don't have mco, trying to add it will cause for exit
with an error

* pao:hypershift: implement the components handler interface

This is the core of the reconciliation loop, which creates/updates/deletes the relevant components.
Similar to what we already have on OCP.

There's a gap about how to retrieve the `containerruntimeconfig` object
which is needed by the controller and is not accessible in the hosted
control plane namespace. This will be handle on a separate PR.

Signed-off-by: Talor Itzhak <[email protected]>

* pao:hypershift: stub implementation for status writer interface

status reporting is not supported on hypershift yet.
we provide a fake implementation in order to conforms the interface.

Signed-off-by: Talor Itzhak <[email protected]>

* pao:hypershift: check if in hypershift first

Checking the feature gate first will result with an error on hypershift since it's not yet supported.

Signed-off-by: Talor Itzhak <[email protected]>

* pao:hypershift: add management client

Adding a management client to be used on a hypershift platform.

Signed-off-by: Talor Itzhak <[email protected]>

* pao:hypershift: configure controller

Add the necessary configuration for the controller, so it would be able to run on a hypershift platform.

1. It needs to configure predicates for ConfigMaps instead of actual objects.
2. It needs to setup different clients for the management cluster and hosted cluster.

Signed-off-by: Talor Itzhak <[email protected]>

* pao:hypershift: generelize reconciliation loop

Modify the reconciliation flow so it could fit for both HCP and OCP.

Signed-off-by: Talor Itzhak <[email protected]>

* e2e:config: adapt suite for hypershift

Adding the relevant code for configuring performanceProfile over hypershift

Signed-off-by: Talor Itzhak <[email protected]>

* hypershift:client: do not assign to nil map

Signed-off-by: Talor Itzhak <[email protected]>

* hypershift:client: return error when configMap has invalid data state

Signed-off-by: Talor Itzhak <[email protected]>

* pao:hypershift: move label validation to helper function

Signed-off-by: Talor Itzhak <[email protected]>

* vendor: bump deps

Bump needed for handling hypershift APIs.
This is the output formatted and generated by the \`go mod tidy\`

Signed-off-by: Talor Itzhak <[email protected]>

* vendor:featuregate: adopt feature gate api changes

The package that holds featuregate geatures has changed in the last vendor bump.
This commit uses the new paths.

Signed-off-by: Talor Itzhak <[email protected]>

* pao:log: small refining

- Move logs to V(4)
- Fix typos
- Remove redundant spacing
- Consistent HyperShift naming

Signed-off-by: Talor Itzhak <[email protected]>

---------

Signed-off-by: Talor Itzhak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants