Skip to content

CORS-2877: Install Config Feature Gate Validation#7413

Merged
openshift-merge-bot[bot] merged 3 commits intoopenshift:masterfrom
patrickdillon:fg-val
Nov 7, 2023
Merged

CORS-2877: Install Config Feature Gate Validation#7413
openshift-merge-bot[bot] merged 3 commits intoopenshift:masterfrom
patrickdillon:fg-val

Conversation

@patrickdillon
Copy link
Contributor

Adds the ability to validate install config fields using feature gates.

Fields can be gated based on any feature set: Default, TechPreviewNoUpgrade, or CustomNoUpgrade.

This means that features can be promoted without changing validation code by vendoring the updated API, where the feature has been promoted to the Default feature set.

Also enables more targeted testing for technical quality.

@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 10, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 10, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@patrickdillon
Copy link
Contributor Author

/test e2e-aws-ovn

@patrickdillon
Copy link
Contributor Author

/test ?

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 10, 2023

@patrickdillon: The following commands are available to trigger required jobs:

  • /test agent-integration-tests
  • /test aro-unit
  • /test e2e-agent-compact-ipv4
  • /test e2e-aws-ovn
  • /test e2e-aws-ovn-upi
  • /test e2e-azure-ovn
  • /test e2e-azure-ovn-upi
  • /test e2e-gcp-ovn
  • /test e2e-gcp-ovn-upi
  • /test e2e-metal-ipi-ovn-ipv6
  • /test e2e-openstack-ovn
  • /test e2e-vsphere-ovn
  • /test e2e-vsphere-upi
  • /test gofmt
  • /test golint
  • /test govet
  • /test images
  • /test okd-images
  • /test okd-scos-images
  • /test okd-unit
  • /test okd-verify-codegen
  • /test openstack-manifests
  • /test shellcheck
  • /test tf-lint
  • /test unit
  • /test verify-codegen
  • /test verify-vendor
  • /test yaml-lint

The following commands are available to trigger optional jobs:

  • /test e2e-agent-compact-ipv4-appliance
  • /test e2e-agent-compact-ipv4-none-platform
  • /test e2e-agent-ha-dualstack
  • /test e2e-agent-sno-ipv4-pxe
  • /test e2e-agent-sno-ipv6
  • /test e2e-alibaba
  • /test e2e-aws-ovn-disruptive
  • /test e2e-aws-ovn-fips
  • /test e2e-aws-ovn-imdsv2
  • /test e2e-aws-ovn-localzones
  • /test e2e-aws-ovn-proxy
  • /test e2e-aws-ovn-public-subnets
  • /test e2e-aws-ovn-shared-vpc
  • /test e2e-aws-ovn-shared-vpc-localzones
  • /test e2e-aws-ovn-single-node
  • /test e2e-aws-ovn-upgrade
  • /test e2e-aws-ovn-workers-rhel8
  • /test e2e-aws-upi-proxy
  • /test e2e-azure-ovn-resourcegroup
  • /test e2e-azure-ovn-shared-vpc
  • /test e2e-azurestack
  • /test e2e-azurestack-upi
  • /test e2e-crc
  • /test e2e-gcp-ovn-shared-vpc
  • /test e2e-gcp-ovn-xpn
  • /test e2e-gcp-secureboot
  • /test e2e-gcp-upgrade
  • /test e2e-gcp-upi-xpn
  • /test e2e-ibmcloud-ovn
  • /test e2e-libvirt
  • /test e2e-metal-assisted
  • /test e2e-metal-ipi-ovn-dualstack
  • /test e2e-metal-ipi-sdn
  • /test e2e-metal-ipi-sdn-swapped-hosts
  • /test e2e-metal-ipi-sdn-virtualmedia
  • /test e2e-metal-single-node-live-iso
  • /test e2e-nutanix-ovn
  • /test e2e-nutanix-sdn
  • /test e2e-openstack-externallb
  • /test e2e-openstack-kuryr
  • /test e2e-openstack-nfv-intel
  • /test e2e-openstack-proxy
  • /test e2e-openstack-sdn-parallel
  • /test e2e-openstack-upi
  • /test e2e-vsphere-static-ovn
  • /test e2e-vsphere-upi-zones
  • /test e2e-vsphere-zones
  • /test okd-e2e-aws-ovn
  • /test okd-e2e-aws-ovn-upgrade
  • /test okd-e2e-gcp
  • /test okd-e2e-gcp-ovn-upgrade
  • /test okd-e2e-vsphere
  • /test okd-scos-e2e-aws-ovn
  • /test okd-scos-e2e-aws-upgrade
  • /test okd-scos-e2e-gcp
  • /test okd-scos-e2e-gcp-ovn-upgrade
  • /test okd-scos-e2e-vsphere
  • /test okd-scos-unit
  • /test okd-scos-verify-codegen
  • /test tf-fmt

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

  • pull-ci-openshift-installer-master-aro-unit
  • pull-ci-openshift-installer-master-e2e-aws-ovn
  • pull-ci-openshift-installer-master-gofmt
  • pull-ci-openshift-installer-master-golint
  • pull-ci-openshift-installer-master-govet
  • pull-ci-openshift-installer-master-images
  • pull-ci-openshift-installer-master-okd-e2e-aws-ovn
  • pull-ci-openshift-installer-master-okd-e2e-aws-ovn-upgrade
  • pull-ci-openshift-installer-master-okd-images
  • pull-ci-openshift-installer-master-okd-scos-e2e-aws-ovn
  • pull-ci-openshift-installer-master-okd-scos-images
  • pull-ci-openshift-installer-master-okd-scos-unit
  • pull-ci-openshift-installer-master-okd-scos-verify-codegen
  • pull-ci-openshift-installer-master-okd-unit
  • pull-ci-openshift-installer-master-okd-verify-codegen
  • pull-ci-openshift-installer-master-shellcheck
  • pull-ci-openshift-installer-master-tf-fmt
  • pull-ci-openshift-installer-master-tf-lint
  • pull-ci-openshift-installer-master-unit
  • pull-ci-openshift-installer-master-verify-codegen
  • pull-ci-openshift-installer-master-verify-vendor
  • pull-ci-openshift-installer-master-yaml-lint
Details

In response to this:

/test ?

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.

@patrickdillon
Copy link
Contributor Author

/test golint
/test gofmt
/test unit

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 16, 2023
@patrickdillon patrickdillon changed the title Install Config Feature Gate Validation CORS-2877: Install Config Feature Gate Validation Oct 19, 2023
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 19, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 19, 2023

@patrickdillon: This pull request references CORS-2877 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.15.0" version, but no target version was set.

Details

In response to this:

Adds the ability to validate install config fields using feature gates.

Fields can be gated based on any feature set: Default, TechPreviewNoUpgrade, or CustomNoUpgrade.

This means that features can be promoted without changing validation code by vendoring the updated API, where the feature has been promoted to the Default feature set.

Also enables more targeted testing for technical quality.

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.

@patrickdillon patrickdillon marked this pull request as ready for review October 30, 2023 22:44
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 30, 2023
@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 Oct 30, 2023
Comment on lines +13 to +21
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@openshift-ci openshift-ci bot requested a review from rvanderp3 October 30, 2023 22:53
Comment on lines +1108 to +1117
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not updating this because it will be removed by #7570, which should merge soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaysaMacedo @EmilienM @pierreprinetti @mandre PTAL for future feature gating needs

@patrickdillon
Copy link
Contributor Author

/cc @JoelSpeed

@openshift-ci openshift-ci bot requested a review from JoelSpeed October 30, 2023 23:01
Adds a featuregate package to allow feature gating
based on openshift/api.

This commit copies code from other repos (acknowledged
in the headers) rather than importing because the sources
are well-contained and we want to minimize imports to
pkg/types as pkg/types is intended to be imported by other
codebases and by doing this we minimize their dependencies.

This code is mostly borrowed from:
https://github.com/openshift/cluster-config-operator/blob/636a2dc303037e2561a243ae1ab5c5b953ddad04/pkg/cmd/render/render.go#L153
Enable checking install config fields with the feature gate package.
Allows install config fields to be validated with CustomNoUpgrade
feature sets in addition to the TechPreviewNoUpgrade feature set.
This commit removes error checking from the GenerateCustomFeatures
function. There are two possible errors that are caught in this
function, and both are already handled separately by validation. So
continually checking for errors throughout the code makes using
feature gate checks less ergonomic for no benefit.

The two potential errors are:

1. Unknown feature set within FeatureGateFromFeatureSets, which is
already validated here:
https://github.com/openshift/installer/blob/b36d7c201e0114aa0e9261cf8ae5b8d68c74bf07/pkg/types/validation/installconfig.go#L1051

2. The other is that a custom feature gate was incorrectly entered,
which is validated here:
https://github.com/openshift/installer/blob/master/pkg/types/validation/installconfig.go#L1103
@JoelSpeed
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2023
@patrickdillon
Copy link
Contributor Author

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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 Nov 7, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD e7b2f51 and 2 for PR HEAD 00a99cb in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 7, 2023

@patrickdillon: 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-gcp-secureboot 00a99cb link false /test e2e-gcp-secureboot
ci/prow/okd-e2e-aws-ovn-upgrade 00a99cb link false /test okd-e2e-aws-ovn-upgrade
ci/prow/e2e-gcp-ovn-xpn 00a99cb link false /test e2e-gcp-ovn-xpn

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-bot openshift-merge-bot bot merged commit 53ce73d into openshift:master Nov 7, 2023
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.

4 participants