Skip to content

Render the CVO manifest with user-specified capabilities from the install-config #5645

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
Merged
openshift-merge-robot merged 2 commits into from
Feb 25, 2022
Merged

Render the CVO manifest with user-specified capabilities from the install-config #5645

openshift-merge-robot merged 2 commits into from
Feb 25, 2022

Conversation

@kirankt
Copy link
Contributor

@kirankt kirankt commented Feb 11, 2022

This PR is depends on: #5605 and #5629.

The CVO manifest is rendered using capabilities specified in the install-config.

@openshift-ci openshift-ci bot requested review from jhixson74 and jstuever February 11, 2022 20:22
@kirankt
Copy link
Contributor Author

kirankt commented Feb 11, 2022

/cc @wking

@openshift-ci openshift-ci bot requested a review from wking February 11, 2022 20:27
@wking
Copy link
Member

wking commented Feb 11, 2022

These are stacking up a bit deep, but looks like this change doesn't actually need to depend on #5629? Hopefully #5605 will land soon, and we can save some time on rebasing.

@kirankt
Copy link
Contributor Author

kirankt commented Feb 21, 2022

/assign @staebler

@kirankt
Copy link
Contributor Author

kirankt commented Feb 21, 2022

/retest

staebler
staebler reviewed Feb 23, 2022
View reviewed changes
Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

Added reviews for all three PRs into this PR.

pkg/types/validation/installconfig.go Outdated
Comment on lines 731 to 732
Copy link
Contributor

@staebler staebler Feb 23, 2022

Choose a reason for hiding this comment

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

If you use a set, then you can use Difference here. This will make this a bit more concise and easier to reason about.

This was referenced Feb 24, 2022
Closed
Closed
staebler
staebler reviewed Feb 24, 2022
View reviewed changes
staebler
staebler reviewed Feb 24, 2022
View reviewed changes
Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 24, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: staebler

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 24, 2022
@wking
Copy link
Member

wking commented Feb 24, 2022

No CI coverage today, so avoid all the non-required flavors:

/skip

@wking
Copy link
Member

wking commented Feb 24, 2022

registry.redhat.io outage took out e2e-aws. But looks like GitHub is complaining about vendor merge conflicts due to some parallel change.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2022
@kirankt
Add user-selectable capabilities api in the installconfig
9126f67 
Fix the explain print output to include capabilities

Validate additional user-specified openshift component capabilities

Render the CVO manifest with user-specified capabilities from the install-config

Address review changes
@openshift-ci openshift-ci bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 24, 2022
staebler
staebler reviewed Feb 25, 2022
View reviewed changes
Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 25, 2022
@wking
Copy link
Member

wking commented Feb 25, 2022

/skip

@openshift-bot
Copy link
Contributor

openshift-bot commented Feb 25, 2022

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 25, 2022

@kirankt: 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/e2e-metal-ipi-ovn-ipv6 2af5c5b link false /test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-openstack 2af5c5b link false /test e2e-openstack
ci/prow/e2e-openstack-kuryr 2af5c5b link false /test e2e-openstack-kuryr
ci/prow/e2e-azure-upi 2af5c5b link false /test e2e-azure-upi
ci/prow/okd-e2e-aws 2af5c5b link false /test okd-e2e-aws
ci/prow/e2e-ibmcloud 2af5c5b link false /test e2e-ibmcloud
ci/prow/okd-e2e-aws-upgrade 2af5c5b link false /test okd-e2e-aws-upgrade
ci/prow/e2e-libvirt 2af5c5b link false /test e2e-libvirt
ci/prow/e2e-crc 2af5c5b link false /test e2e-crc

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/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit a45282c into openshift:master Feb 25, 2022
@wking wking mentioned this pull request Feb 26, 2022
Merged
wking added a commit to wking/openshift-release that referenced this pull request Mar 3, 2022
@wking
ci-operator/config/openshift/release: Add no-capabilities periodic
44c0c66 
Installing a cluster using the new knobs the installer grew in [1,2].

[1]: openshift/installer#5645
[2]: https://github.com/openshift/enhancements/blob/5d694be26311d2d2683b85a72376919c625854d0/enhancements/installer/component-selection.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhixson74 jhixson74 Awaiting requested review from jhixson74

@jstuever jstuever Awaiting requested review from jstuever

@wking wking Awaiting requested review from wking

1 more reviewer

@staebler staebler staebler left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@staebler staebler

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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.

5 participants

@kirankt @wking @openshift-bot @staebler @openshift-merge-robot