Skip to content

Comments

ARO-14442: introduce capabilities api#5554

Merged
openshift-merge-bot[bot] merged 4 commits intoopenshift:mainfrom
flavianmissi:capabilities-api-ARO-14442
Feb 6, 2025
Merged

ARO-14442: introduce capabilities api#5554
openshift-merge-bot[bot] merged 4 commits intoopenshift:mainfrom
flavianmissi:capabilities-api-ARO-14442

Conversation

@flavianmissi
Copy link
Member

@flavianmissi flavianmissi commented Feb 4, 2025 •
edited
Loading

What this PR does / why we need it:
We're introducing the concept of capabilities to Hypershift on a limited basis, as described in openshift/enhancements#1729.

Which issue(s) this PR fixes *:
Fixes ARO-14442

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 4, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 4, 2025 •
edited by openshift-ci bot
Loading

@flavianmissi: This pull request references ARO-14442 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.19.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:
We're introducing the concept of capabilities to Hypershift on a limited basis, as described in openshift/enhancements#1729.

Which issue(s) this PR fixes *:
Fixes ARO-14442

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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

openshift-ci-robot commented Feb 4, 2025 •
edited by openshift-ci bot
Loading

@flavianmissi: This pull request references ARO-14442 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.19.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:
We're introducing the concept of capabilities to Hypershift on a limited basis, as described in openshift/enhancements#1729.

Which issue(s) this PR fixes *:
Fixes ARO-14442

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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 area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/documentation Indicates the PR includes changes for documentation area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release and removed do-not-merge/needs-area labels Feb 4, 2025
enxebre
enxebre reviewed Feb 4, 2025
View reviewed changes
enxebre
enxebre reviewed Feb 4, 2025
View reviewed changes
api/hypershift/v1beta1/hostedcluster_types.go Outdated
// +listType=atomic
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="disabledCapabilities is immutable"
// +optional
// +openshift:enable:FeatureGate=DisableClusterCapabilities
Copy link
Member

@enxebre enxebre Feb 4, 2025

Choose a reason for hiding this comment

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

should we add min/max?

Copy link
Member Author

@flavianmissi flavianmissi Feb 5, 2025

Choose a reason for hiding this comment

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

Right, thanks for reminding me about this. For now we only have one valid value: ImageRegistry, so should the min/max both be 13 (len(ImageRegistry))? Or are there some sensible defaults we can use instead?

@flavianmissi flavianmissi force-pushed the capabilities-api-ARO-14442 branch from ffbc00c to b069ed6 Compare February 5, 2025 11:14
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2025
@openshift-ci openshift-ci bot added the area/testing Indicates the PR includes changes for e2e testing label Feb 5, 2025
@flavianmissi flavianmissi force-pushed the capabilities-api-ARO-14442 branch 2 times, most recently from 772c4e4 to 580f3da Compare February 6, 2025 09:29
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 6, 2025
@flavianmissi flavianmissi force-pushed the capabilities-api-ARO-14442 branch from 580f3da to 73ee4ba Compare February 6, 2025 09:40
@flavianmissi
ARO-14442: add DisabledCapabilities to HostedClusterSpec
a07c734 
* api: feature gate DisabledCapabilities field on HostedCluster
* api: add DisabledCapabilities to HostedControlPlaneSpec

Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
Signed-off-by: Flavian Missi <fmissi@redhat.com>
@flavianmissi
regenerate files
7ae31e1 
run `make api` to regenerate files and `make update` to bump
hypershift/api dep.
@flavianmissi flavianmissi force-pushed the capabilities-api-ARO-14442 branch from 73ee4ba to 7ae31e1 Compare February 6, 2025 09:50
@enxebre
Copy link
Member

enxebre commented Feb 6, 2025

/approve

@enxebre
Copy link
Member

enxebre commented Feb 6, 2025

ptal @sjenning

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 6, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre, flavianmissi

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

The pull request process is described here

Details 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 Feb 6, 2025
@flavianmissi flavianmissi mentioned this pull request Feb 6, 2025
Merged
4 tasks
@flavianmissi flavianmissi mentioned this pull request Feb 6, 2025
Closed
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 6, 2025 •
edited by openshift-ci bot
Loading

@flavianmissi: This pull request references ARO-14442 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.19.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:
We're introducing the concept of capabilities to Hypershift on a limited basis, as described in openshift/enhancements#1729.

Which issue(s) this PR fixes *:
Fixes ARO-14442

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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

openshift-ci-robot commented Feb 6, 2025 •
edited by openshift-ci bot
Loading

@flavianmissi: This pull request references ARO-14442 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.19.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:
We're introducing the concept of capabilities to Hypershift on a limited basis, as described in openshift/enhancements#1729.

Which issue(s) this PR fixes *:
Fixes ARO-14442

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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.

@flavianmissi
Copy link
Member Author

flavianmissi commented Feb 6, 2025

I don't see how the test failures are connected to the changes 🤔

/retest

@enxebre
Copy link
Member

enxebre commented Feb 6, 2025

/test e2e-aws

@sjenning
Copy link
Contributor

sjenning commented Feb 6, 2025

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 6, 2025
@flavianmissi
Copy link
Member Author

flavianmissi commented Feb 6, 2025

from latest e2e-aws failure:

eventually.go:223: observed *v1beta1.HostedCluster e2e-clusters-rjbch/example-9xpt7 invalid at RV 46558 after 10m0s: incorrect condition: wanted AWSEndpointAvailable=True, got AWSEndpointAvailable=False: AWSError(failed to create vpc endpoint: RequestLimitExceeded)

unsure if any action is needed. It'll probably clear up in the next few hours.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 6, 2025

/retest-required

Remaining retests: 0 against base HEAD 91bb067 and 2 for PR HEAD 7ae31e1 in total

@sjenning
Copy link
Contributor

sjenning commented Feb 6, 2025

This has flaked a few times in a row. I'm confident is passes.

/override ci/prow/e2e-aws

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 6, 2025

@sjenning: Overrode contexts on behalf of sjenning: ci/prow/e2e-aws

Details

In response to this:

This has flaked a few times in a row. I'm confident is passes.

/override ci/prow/e2e-aws

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 6, 2025

@flavianmissi: The following test 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/okd-scos-e2e-aws-ovn 7ae31e1 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

Details

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 dd5a61d into openshift:main Feb 6, 2025
13 of 14 checks passed
@openshift-bot
Copy link

openshift-bot commented Feb 7, 2025

[ART PR BUILD NOTIFIER]

Distgit: hypershift
This PR has been included in build ose-hypershift-container-v4.19.0-202502070007.p0.gdd5a61d.assembly.stream.el9.
All builds following this will include this PR.

@flavianmissi flavianmissi deleted the capabilities-api-ARO-14442 branch February 7, 2025 07:09
@flavianmissi
Copy link
Member Author

flavianmissi commented Apr 1, 2025

/cherry-pick release-4.18

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Apr 1, 2025

@flavianmissi: #5554 failed to apply on top of branch "release-4.18":

Applying: hypershift-operator: gofumpt hostedcluster controller
Applying: hypershift-operator: add capabilities feature gate
Using index info to reconstruct a base tree...
M	api/hypershift/v1beta1/featuregates/featureGate-Hypershift-Default.yaml
M	api/hypershift/v1beta1/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
M	api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-Default.yaml
M	api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
M	cmd/install/assets/hypershift_operator_test.go
Falling back to patching base and 3-way merge...
Auto-merging cmd/install/assets/hypershift_operator_test.go
Auto-merging api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
CONFLICT (content): Merge conflict in api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
Auto-merging api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-Default.yaml
CONFLICT (content): Merge conflict in api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-Default.yaml
Auto-merging api/hypershift/v1beta1/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
CONFLICT (content): Merge conflict in api/hypershift/v1beta1/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
Auto-merging api/hypershift/v1beta1/featuregates/featureGate-Hypershift-Default.yaml
CONFLICT (content): Merge conflict in api/hypershift/v1beta1/featuregates/featureGate-Hypershift-Default.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0002 hypershift-operator: add capabilities feature gate
Details

In response to this:

/cherry-pick release-4.18

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.

@flavianmissi flavianmissi mentioned this pull request Apr 2, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@enxebre enxebre enxebre left review comments

@isco-rodriguez isco-rodriguez Awaiting requested review from isco-rodriguez

@sjenning sjenning Awaiting requested review from sjenning

Assignees

@sjenning sjenning

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/documentation Indicates the PR includes changes for documentation area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/testing Indicates the PR includes changes for e2e testing 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@flavianmissi @openshift-ci-robot @enxebre @sjenning @openshift-bot @openshift-cherrypick-robot @openshift-merge-robot