openshift / api

Canonical location of the OpenShift API definition.
http://www.openshift.org
Apache License 2.0
94 stars 517 forks source link

CORS-3741: [Nutanix] allow multi-subnets in Machine providerSpec and failureDomain configuration #2077

Closed yanhua121 closed 1 day ago

yanhua121 commented 3 weeks ago

CORS-3741 Nutanix: allow multi-subnets in Machine providerSpec and failureDomain configuration

openshift-ci[bot] commented 3 weeks ago

Hello @yanhua121! Some important instructions when contributing to openshift/api: API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

yanhua121 commented 3 weeks ago

/retest

yanhua121 commented 3 weeks ago

/assign @JoelSpeed

openshift-ci-robot commented 3 weeks ago

@yanhua121: An error was encountered searching for bug CORS-3741 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message. No response returned: Get "https://issues.redhat.com/rest/api/2/issue/CORS-3741": GET https://issues.redhat.com/rest/api/2/issue/CORS-3741 giving up after 5 attempt(s)

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to [this](https://github.com/openshift/api/pull/2077): >Nutanix: allow multi-subnets in Machine providerSpec and failureDomain configuration Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fapi). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 3 weeks ago

@yanhua121: An error was encountered searching for bug CORS-3741 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message. No response returned: Get "https://issues.redhat.com/rest/api/2/issue/CORS-3741": GET https://issues.redhat.com/rest/api/2/issue/CORS-3741 giving up after 5 attempt(s)

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to [this](https://github.com/openshift/api/pull/2077): >CORS-3741 >Nutanix: allow multi-subnets in Machine providerSpec and failureDomain configuration Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fapi). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
JoelSpeed commented 2 weeks ago

New feature must now be implemented behind featuregates, especially this close to branching.

Also, validation ratcheting is a new feature, so we will need to test the ratcheting feature for the new failure domains limit, for now, lets see if we can ask QE to create a cluster, add 257 failure domains, and then upgrade to this new schema and observe what happens

JoelSpeed commented 2 weeks ago

Existing schema check errors cannot be fixed and will be overridden when ready, in particular, see the last line of this block

error running generator schemacheck on group machineconfiguration.openshift.io: 
    could not run schemacheck generator for group/version machineconfiguration.openshift.io/v1: 
        error in machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yaml: ConditionsMustHaveProperSSATags: crd/controllerconfigs.machineconfiguration.openshift.io version/v1 field/^.status.condition must define valid condition properties: observedGeneration attribute is missing
        error in machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yaml: NoMaps: crd/controllerconfigs.machineconfiguration.openshift.io version/v1 field/^.spec.dns.spec.privateZone.tags may not be a map
        error in machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yaml: NoMaps: crd/controllerconfigs.machineconfiguration.openshift.io version/v1 field/^.spec.dns.spec.publicZone.tags may not be a map
        error in machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yaml: NoMaps: crd/controllerconfigs.machineconfiguration.openshift.io version/v1 field/^.spec.images may not be a map
echo This verifier checks all files that have changed. In some cases you may have changed or renamed a file that already contained api violations, but you are not introducing a new violation. In such cases it is appropriate to /override the failing CI job.
openshift-ci[bot] commented 2 weeks ago

@yanhua121: yanhua121 unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight openshift-staff-engineers.

In response to [this](https://github.com/openshift/api/pull/2077#issuecomment-2462582406): >/override ci/prow/verify-crd-schema ci/prow/verify Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
JoelSpeed commented 1 week ago

Verify failing legitimately,

No test suite file found for CRD /go/src/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/NutanixMultiSubnets.yaml, expected /go/src/github.com/openshift/api/config/v1/tests/infrastructures.config.openshift.io/NutanixMultiSubnets.yaml

We need tests for the new validations we have added in the test suite file at this path

Test documentation: https://github.com/openshift/api/blob/master/tests/README.md

yanhua121 commented 1 week ago

Verify failing legitimately,

No test suite file found for CRD /go/src/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/NutanixMultiSubnets.yaml, expected /go/src/github.com/openshift/api/config/v1/tests/infrastructures.config.openshift.io/NutanixMultiSubnets.yaml

We need tests for the new validations we have added in the test suite file at this path

Test documentation: https://github.com/openshift/api/blob/master/tests/README.md

Okay, will add test cases for the newly added featuregate validation rules.

yanhua121 commented 1 week ago

/retest

yanhua121 commented 1 week ago

/assign @deads2k

yanhua121 commented 3 days ago

Verify failing legitimately,

No test suite file found for CRD /go/src/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests/infrastructures.config.openshift.io/NutanixMultiSubnets.yaml, expected /go/src/github.com/openshift/api/config/v1/tests/infrastructures.config.openshift.io/NutanixMultiSubnets.yaml

We need tests for the new validations we have added in the test suite file at this path

Test documentation: https://github.com/openshift/api/blob/master/tests/README.md

@JoelSpeed @elmiko The featuregate reuqired tests are added. Can you override the test ci/prow/verify-crd-schema? Because the test failures are expected for a newly added feature gate.

elmiko commented 3 days ago

i talked with @deads2k on slack, it might be possible for us to clean up this error from the failed test:

error in machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yaml: ConditionsMustHaveProperSSATags: crd/controllerconfigs.machineconfiguration.openshift.io version/v1 field/^.status.condition must define valid condition properties: observedGeneration attribute is missing

but i will investigate adding that in a different PR

JoelSpeed commented 3 days ago

i talked with @deads2k on slack, it might be possible for us to clean up this error from the failed test:

error in machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yaml: ConditionsMustHaveProperSSATags: crd/controllerconfigs.machineconfiguration.openshift.io version/v1 field/^.status.condition must define valid condition properties: observedGeneration attribute is missing

but i will investigate adding that in a different PR

This issue cannot be resolved without introducing a new observed generation field to the status of all operators in OpenShift, and that's not a change I'm willing to risk on a whim, it's well out of scope for this PR

The issue specifically about observedGeneration being missing can be ignored

JoelSpeed commented 3 days ago

All failures in the schema-check cannot presently be fixed

/override ci/prow/verify-crd-schema

/lgtm

Lets continue working on the EP to make sure we understand the testing we want in place to promote this feature

openshift-ci[bot] commented 3 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, yanhua121

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: - ~~[OWNERS](https://github.com/openshift/api/blob/master/OWNERS)~~ [JoelSpeed] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-ci[bot] commented 3 days ago

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema

In response to [this](https://github.com/openshift/api/pull/2077#issuecomment-2484272967): >All failures in the [schema-check](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_api/2077/pull-ci-openshift-api-master-verify-crd-schema/1858631219135123456) cannot presently be fixed > >/override ci/prow/verify-crd-schema > >/lgtm > >Lets continue working on the EP to make sure we understand the testing we want in place to promote this feature Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
openshift-ci-robot commented 3 days ago

/retest-required

Remaining retests: 0 against base HEAD 25d2eecae482743bb3bbb30e0e6a34a8bcdb1a36 and 2 for PR HEAD 09b63162bb151d7fa27fc063162f2d2b4cf197a0 in total

openshift-ci-robot commented 2 days ago

/retest-required

Remaining retests: 0 against base HEAD 25d2eecae482743bb3bbb30e0e6a34a8bcdb1a36 and 2 for PR HEAD 09b63162bb151d7fa27fc063162f2d2b4cf197a0 in total

yanhua121 commented 2 days ago

/retest-required

openshift-ci-robot commented 2 days ago

/retest-required

Remaining retests: 0 against base HEAD 25d2eecae482743bb3bbb30e0e6a34a8bcdb1a36 and 2 for PR HEAD 09b63162bb151d7fa27fc063162f2d2b4cf197a0 in total

openshift-ci-robot commented 2 days ago

/retest-required

Remaining retests: 0 against base HEAD 25d2eecae482743bb3bbb30e0e6a34a8bcdb1a36 and 2 for PR HEAD 09b63162bb151d7fa27fc063162f2d2b4cf197a0 in total

JoelSpeed commented 2 days ago

/override ci/prow/verify-crd-schema

openshift-ci[bot] commented 2 days ago

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema

In response to [this](https://github.com/openshift/api/pull/2077#issuecomment-2486662770): >/override ci/prow/verify-crd-schema Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
yanhua121 commented 2 days ago

/retest-required

yanhua121 commented 2 days ago

/label acknowledge-critical-fixes-only

yanhua121 commented 2 days ago

/test e2e-aws-serial-techpreview

openshift-ci[bot] commented 1 day ago

@yanhua121: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
openshift-bot commented 1 day ago

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-config-api This PR has been included in build ose-cluster-config-api-container-v4.19.0-202411200908.p0.gcaf9796.assembly.stream.el9. All builds following this will include this PR.