STOR-1839: promote VSphereDriverConfiguration feature to Accessible-by-default #1902

RomanBednar · 2024-05-22T09:24:23Z

openshift-ci-robot · 2024-05-22T09:24:26Z

@RomanBednar: This pull request references STOR-1839 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.17.0" version, but no target version was set.

In response to this:

...and remove the feature gate since it's not used in our code.

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 · 2024-05-22T09:24:36Z

Hello @RomanBednar! 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.

JoelSpeed · 2024-05-22T09:42:51Z

features/features.go

-	FeatureGateVSphereDriverConfiguration = newFeatureGate("VSphereDriverConfiguration").
-						reportProblemsToJiraComponent("Storage / Kubernetes External Components").
-						contactPerson("rbednar").
-						productScope(ocpSpecific).
-						enableIn(configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade).
-						mustRegister()
-


For reasons beyond this PR, you can't actually remove gates at present. This must just be promoted to default and not removed. Gates will be cleaned up in a future release once they have been enabled by default for at least 2 EUS upgrades

I see, is this documented somewhere for future myself? I was looking at this: https://github.com/openshift/enhancements/blob/b99a80d976385faa87f251dbbcd4260a37406921/dev-guide/featuresets.md#steps-to-remove-a-feature-gate

Which says the feature gate can be removed once it's no longer used, and this one in particular is not used currently. Also mentions removing if clauses for feature gates In the next release -> so it's actually 2 EUS upgrades as you said. Is the doc outdated perhaps?

Yeah the doc is in need of an update, we were discussing this on yesterdays office hours call, will be in the queue to update soon

JoelSpeed · 2024-05-22T09:43:28Z

operator/v1/tests/clustercsidrivers.operator.openshift.io/VSphereDriverConfiguration.yaml

-  - name: Should be able to create a minimal ClusterCSIDriver
-    initial: |
-      apiVersion: operator.openshift.io/v1
-      kind: ClusterCSIDriver
-      metadata:
-        name: csi.sharedresource.openshift.io
-      spec: {} # No spec is required for a ClusterCSIDriver
-    expected: |
-      apiVersion: operator.openshift.io/v1
-      kind: ClusterCSIDriver
-      metadata:
-        name: csi.sharedresource.openshift.io
-      spec:
-        logLevel: Normal
-        operatorLogLevel: Normal
-  - name: IBM Cloud CSIDriverType must have a defined IBM Cloud spec
-    initial: |
-      apiVersion: operator.openshift.io/v1
-      kind: ClusterCSIDriver
-      metadata:
-        name: csi.sharedresource.openshift.io
-      spec:
-        driverConfig:
-          driverType: IBMCloud
-    expectedError: "Invalid value: \"object\": ibmcloud must be set if driverType is 'IBMCloud', but remain unset otherwise"
-  - name: IBM Cloud spec must have an EncryptionKeyCRN defined
-    initial: |
-      apiVersion: operator.openshift.io/v1
-      kind: ClusterCSIDriver
-      metadata:
-        name: csi.sharedresource.openshift.io
-      spec:
-        driverConfig:
-          driverType: IBMCloud
-          ibmcloud: {}
-    expectedError: "spec.driverConfig.ibmcloud.encryptionKeyCRN: Required value, <nil>: Invalid value: \"null\": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation"


These tests are already present in the default set of tests?

Yes, but the were duplicated in this file which is for the feature gate only I suppose - anyway, not applicable to latest version of this PR since it's not getting removed.

JoelSpeed · 2024-05-22T09:43:39Z

operator/v1/tests/clustercsidrivers.operator.openshift.io/AAA_ungated.yaml

+              granularMaxSnapshotsPerBlockVolumeInVSAN: 2
+              granularMaxSnapshotsPerBlockVolumeInVVOL: 3
+          logLevel: Normal
+          operatorLogLevel: Normal


Nit, please restore the new line char at the end of the file

Resolved - this does not have to change it seems.

openshift-ci-robot · 2024-05-22T10:12:40Z

@RomanBednar: This pull request references STOR-1839 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.17.0" version, but no target version was set.

In response to this:

4.16 Periodic runs

E2E Tests 4.16 (Sippy)

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.

RomanBednar · 2024-05-22T11:19:42Z

@JoelSpeed My e2e test don't have [OCPFeatureGate:VSphereDriverConfiguration] or [FeatureGate:VSphereDriverConfiguration] but [Feature:VSphereDriverConfiguration] since I was not aware of this requirement before, which is why the verify job fails. Does the test have to be renamed or can we override this? I've linked the tests in PR description.

And verify-crd-schema job error .spec.driverConfig.vSphere.topologyCategories must set x-kubernetes-list-type -> this was not changed in scope of this PR, should it be fixed?

JoelSpeed · 2024-05-22T11:30:34Z

Provided tests are linked and/or QE approval is marked on the PR, we can override the test naming clauses yes. This is a new thing so we are easing features into it as they come

RomanBednar · 2024-05-22T11:54:24Z

/override ci/prow/verify

openshift-ci · 2024-05-22T11:55:53Z

@RomanBednar: RomanBednar 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:

/override ci/prow/verify

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.

JoelSpeed · 2024-05-22T12:07:26Z

error running generator schemacheck on group operator.openshift.io: 
	could not run schemacheck generator for group/version operator.openshift.io/v1: 
		error in operator/v1/zz_generated.crd-manifests/0000_90_csi-driver_01_clustercsidrivers.crd.yaml: ListsMustHaveSSATags: crd/clustercsidrivers.operator.openshift.io version/v1 field/^.spec.driverConfig.vSphere.topologyCategories must set x-kubernetes-list-type

Can you please fix this? // +listType=atomic is the current behaviour and since it's just a list of strings, that just needs to be added in to resolve the issue

Phaow · 2024-05-22T12:18:36Z

/label qe-approved
The e2e history looks good and the feature is stable enough to gradutate to GA.

openshift-ci-robot · 2024-05-22T12:18:42Z

@RomanBednar: This pull request references STOR-1839 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.17.0" version, but no target version was set.

In response to this:

4.16 Periodic runs

E2E Tests 4.16 (Sippy)

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.

RomanBednar · 2024-05-22T12:28:12Z

Can you please fix this? // +listType=atomic is the current behaviour and since it's just a list of strings, that just needs to be added in to resolve the issue

@JoelSpeed Sure, fixed in separate commit.

JoelSpeed · 2024-05-22T16:15:56Z

Based on linked content in the PR description, looks like this is working and the tests are there, though the name pattern doesn't match the verify expectation, so will override
/override ci/prow/verify

/lgtm

We are working on fixing the client-go issue, once that's fixed we will look to get this merged

openshift-ci · 2024-05-22T16:32:57Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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~~ [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 · 2024-05-22T16:35:56Z

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

In response to this:

Based on linked content in the PR description, looks like this is working and the tests are there, though the name pattern doesn't match the verify expectation, so will override
/override ci/prow/verify

/lgtm

We are working on fixing the client-go issue, once that's fixed we will look to get this merged

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-robot · 2024-05-22T17:16:25Z

/retest-required

Remaining retests: 0 against base HEAD 0314f31 and 2 for PR HEAD 08e8144 in total

RomanBednar · 2024-05-23T13:51:13Z

/retest-required

openshift-ci · 2024-05-23T14:03:28Z

@RomanBednar: 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.

RomanBednar · 2024-05-23T15:04:34Z

/cherry-pick release-4.16

openshift-cherrypick-robot · 2024-05-23T15:05:31Z

@RomanBednar: new pull request created: #1904

In response to this:

/cherry-pick release-4.16

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-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 22, 2024

openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 22, 2024

RomanBednar changed the title ~~STOR-1839: promote feature from Tech Preview to Accessible-by-default~~ STOR-1839: promote VSphereDriverConfiguration feature from Tech Preview to Accessible-by-default May 22, 2024

RomanBednar changed the title ~~STOR-1839: promote VSphereDriverConfiguration feature from Tech Preview to Accessible-by-default~~ STOR-1839: promote VSphereDriverConfiguration feature to Accessible-by-default May 22, 2024

openshift-ci bot requested review from derekwaynecarr and JoelSpeed May 22, 2024 09:26

JoelSpeed reviewed May 22, 2024

View reviewed changes

RomanBednar added 2 commits May 22, 2024 12:09

enable VSphereDriverConfiguration feature gate by default

6bdeb80

make update

b7debf5

RomanBednar force-pushed the driver-config-ga branch from 087922f to b7debf5 Compare May 22, 2024 10:10

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label May 22, 2024

add listType marker to TopologyCategories

08e8144

openshift-ci bot assigned JoelSpeed May 22, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 22, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 22, 2024

openshift-merge-bot bot merged commit 4bdfeed into openshift:master May 23, 2024
18 checks passed

RomanBednar mentioned this pull request May 23, 2024

STOR-1839: VSphereDriverConfiguration feature Accessible-by-default openshift/cluster-storage-operator#475

Merged

openshift-cherrypick-robot mentioned this pull request May 23, 2024

[release-4.16] STOR-1839: promote VSphereDriverConfiguration feature to Accessible-by-default #1904

Merged

RomanBednar mentioned this pull request May 31, 2024

STOR-1839: run vsphere driver config test on non techpreview clusters openshift/origin#28848

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STOR-1839: promote VSphereDriverConfiguration feature to Accessible-by-default #1902

STOR-1839: promote VSphereDriverConfiguration feature to Accessible-by-default #1902

RomanBednar commented May 22, 2024 •

edited

Loading

openshift-ci-robot commented May 22, 2024 •

edited by openshift-ci bot

Loading

openshift-ci bot commented May 22, 2024

JoelSpeed May 22, 2024

RomanBednar May 22, 2024

JoelSpeed May 22, 2024

JoelSpeed May 22, 2024

RomanBednar May 22, 2024

JoelSpeed May 22, 2024

RomanBednar May 22, 2024

openshift-ci-robot commented May 22, 2024 •

edited by openshift-ci bot

Loading

RomanBednar commented May 22, 2024

JoelSpeed commented May 22, 2024

RomanBednar commented May 22, 2024

openshift-ci bot commented May 22, 2024

JoelSpeed commented May 22, 2024

Phaow commented May 22, 2024

openshift-ci-robot commented May 22, 2024 •

edited by openshift-ci bot

Loading

RomanBednar commented May 22, 2024

JoelSpeed commented May 22, 2024

openshift-ci bot commented May 22, 2024

openshift-ci bot commented May 22, 2024

openshift-ci-robot commented May 22, 2024

RomanBednar commented May 23, 2024

openshift-ci bot commented May 23, 2024

RomanBednar commented May 23, 2024

openshift-cherrypick-robot commented May 23, 2024

STOR-1839: promote VSphereDriverConfiguration feature to Accessible-by-default #1902

STOR-1839: promote VSphereDriverConfiguration feature to Accessible-by-default #1902

Conversation

RomanBednar commented May 22, 2024 • edited Loading

openshift-ci-robot commented May 22, 2024 • edited by openshift-ci bot Loading

openshift-ci bot commented May 22, 2024

JoelSpeed May 22, 2024

Choose a reason for hiding this comment

RomanBednar May 22, 2024

Choose a reason for hiding this comment

JoelSpeed May 22, 2024

Choose a reason for hiding this comment

JoelSpeed May 22, 2024

Choose a reason for hiding this comment

RomanBednar May 22, 2024

Choose a reason for hiding this comment

JoelSpeed May 22, 2024

Choose a reason for hiding this comment

RomanBednar May 22, 2024

Choose a reason for hiding this comment

openshift-ci-robot commented May 22, 2024 • edited by openshift-ci bot Loading

RomanBednar commented May 22, 2024

JoelSpeed commented May 22, 2024

RomanBednar commented May 22, 2024

openshift-ci bot commented May 22, 2024

JoelSpeed commented May 22, 2024

Phaow commented May 22, 2024

openshift-ci-robot commented May 22, 2024 • edited by openshift-ci bot Loading

RomanBednar commented May 22, 2024

JoelSpeed commented May 22, 2024

openshift-ci bot commented May 22, 2024

openshift-ci bot commented May 22, 2024

openshift-ci-robot commented May 22, 2024

RomanBednar commented May 23, 2024

openshift-ci bot commented May 23, 2024

RomanBednar commented May 23, 2024

openshift-cherrypick-robot commented May 23, 2024

RomanBednar commented May 22, 2024 •

edited

Loading

openshift-ci-robot commented May 22, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 22, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented May 22, 2024 •

edited by openshift-ci bot

Loading