Skip to content

Conversation

@stephenfin
Copy link
Contributor

@stephenfin stephenfin commented Sep 17, 2024

openshift/openstack-cinder-csi-driver-operator#180 will mark the old location as obsolete.

openshift-eng/ocp-build-data#5402 will ensure we ship containers from this location.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 17, 2024
@stephenfin stephenfin changed the title OSASINFRA 3608: Use csi-operator for building openstack-cinder CSI driver OSASINFRA-3609: Use csi-operator for building openstack-cinder CSI driver Sep 18, 2024
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 18, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 18, 2024

@stephenfin: This pull request references OSASINFRA-3609 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.18.0" version, but no target version was set.

Details

In response to this:

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-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 18, 2024
stephenfin added a commit to shiftstack/openstack-cinder-csi-driver-operator that referenced this pull request Sep 18, 2024
We are in the process of migrating everything across to csi-operator.
See [1] and [2] for more context.

This is based on same note from aws-ebs-csi-driver-operator repo.

[1]: openshift/csi-operator#278
[2]: openshift/release#56812

Signed-off-by: Stephen Finucane <[email protected]>
@stephenfin
Copy link
Contributor Author

/cc @gnufied
/cc @mandre

@openshift-ci openshift-ci bot requested review from gnufied and mandre September 18, 2024 13:45
@mandre
Copy link
Member

mandre commented Sep 18, 2024

Depends on openshift/csi-operator#278

Copy link
Member

@mandre mandre left a comment

Choose a reason for hiding this comment

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

We shouldn't merge before openshift/csi-operator#278 lands. LGTM otherwise

env:
TEST_SCENARIOS: \[CSI-Driver\]|\[Azure-File-CSI-Driver\]|\[Azure-File-CSI-Driver-Operator\]
workflow: openshift-e2e-azure-csi-extended
- as: e2e-openstack-cinder
Copy link
Member

Choose a reason for hiding this comment

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

I know we're just porting whatever was in the cinder-csi-driver-operator repo, but What's this value of this job, if we have another one that runs CSI specific tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't a clue, though I do note that we currently do similar things for AWS, Azure, etc.

Maybe a question for @gnufied and @jsafrane?

Copy link
Member

Choose a reason for hiding this comment

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

that's possibly to ensure that the operator respects the openshift requirements for their operators, but perhaps that should be part of the suite we run in the openshift-e2e-openstack-csi-cinder workflow.

Copy link
Member

Choose a reason for hiding this comment

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

How is this job different from e2e-openstack-cinder-csi ?

Copy link
Member

Choose a reason for hiding this comment

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

it should be just e2e-openstack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed the job but kept it for now. Let's determine whether we want to keep it long-term post-merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

All components that are installed by default should run regular e2e test to make sure that a CSI driver update does not break other functionality of the cluster, dunno... Routes or Networking. The probability is IMO very small here.

And the same for upgrades - all components that are installed by default should run regular e2e-upgrade job to ensure a CSI driver change does not break upgrade. That's much more likely to happen, we had cases when a driver upgrade broke an existing cluster.

These rules were documented somewhere, but I can't find it now :-(

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 18, 2024

@stephenfin: This pull request references OSASINFRA-3609 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.18.0" version, but no target version was set.

Details

In response to this:

openshift/openstack-cinder-csi-driver-operator#180 will mark the old location as obsolete.

openshift-eng/ocp-build-data#5402 will ensure we ship containers from this location.

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.

@stephenfin stephenfin force-pushed the OSASINFRA-3608 branch 4 times, most recently from 3cfd3b7 to 813ada5 Compare September 19, 2024 17:35
TEST_SUITE: experimental/reliability/minimal
workflow: openshift-e2e-openstack-ipi
- as: e2e-openstack-cinder-csi
run_if_changed: ^(Dockerfile\.openstack-cinder|legacy/openstack-cinder-csi-driver-operator/.*)
Copy link
Member

Choose a reason for hiding this comment

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

nice. this would be useful for other operators too!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may need to be a bit more involved, since conceivably any chance to the "common" code could affect an individual driver. However, we could just rely on the fact that (a) changes to the asset generation will require an asset generation run (I assume there's a job to check that - I have yet to look) and (b) if it's a programming error the build will likely fail. In any case, a can to kick down the road slightly until we start reworking this driver to take advantage of the generator.

@stephenfin
Copy link
Contributor Author

/pj-rehearse pull-ci-openshift-csi-operator-master-e2e-openstack pull-ci-openshift-csi-operator-master-e2e-openstack-cinder-csi pull-ci-openshift-csi-operator-release-4.18-e2e-openstack pull-ci-openshift-csi-operator-release-4.18-e2e-openstack-cinder-csi pull-ci-openshift-csi-operator-release-4.19-e2e-openstack pull-ci-openshift-csi-operator-release-4.19-e2e-openstack-cinder-csi

@openshift-ci-robot
Copy link
Contributor

@stephenfin: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

Make it more obvious what goes where.

Signed-off-by: Stephen Finucane <[email protected]>
Using the current legacy package layout. A future change will expand
this to cover the future package layout.

Signed-off-by: Stephen Finucane <[email protected]>
@openshift-ci-robot
Copy link
Contributor

[REHEARSALNOTIFIER]
@stephenfin: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-openshift-cloud-provider-openstack-master-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.19-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.18-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.17-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.16-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.15-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.14-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.13-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.12-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.11-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.10-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-cloud-provider-openstack-release-4.9-e2e-openstack-csi-cinder openshift/cloud-provider-openstack presubmit Registry content changed
pull-ci-openshift-kubernetes-master-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.19-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.18-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.17-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.16-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.15-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.14-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.13-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.12-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.11-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.10-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-kubernetes-release-4.9-e2e-openstack-csi-cinder openshift/kubernetes presubmit Registry content changed
pull-ci-openshift-csi-operator-master-e2e-openstack openshift/csi-operator presubmit Presubmit changed

A total of 124 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs.

A full list of affected jobs can be found here

Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse abort to abort all active rehearsals

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@mandre
Copy link
Member

mandre commented Sep 20, 2024

/pj-rehearse pull-ci-openshift-csi-operator-master-e2e-openstack-cinder-csi

@openshift-ci-robot
Copy link
Contributor

@mandre: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

Copy link
Member

@mandre mandre 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 Sep 20, 2024
@stephenfin
Copy link
Contributor Author

stephenfin commented Sep 20, 2024

This is looking good. There are failures but the actual deployment happened as expected (which is the important bit)

@stephenfin
Copy link
Contributor Author

/pj-rehearse ack

@openshift-ci-robot
Copy link
Contributor

@stephenfin: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@gnufied
Copy link
Member

gnufied commented Sep 20, 2024

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 20, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, mandre, stephenfin

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 Sep 20, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 20, 2024

@stephenfin: 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/rehearse/openshift/csi-operator/release-4.18/e2e-openstack-cinder-csi 48ebb50 link unknown /pj-rehearse pull-ci-openshift-csi-operator-release-4.18-e2e-openstack-cinder-csi
ci/rehearse/openshift/csi-operator/master/e2e-openstack-cinder-csi a81fc34 link unknown /pj-rehearse pull-ci-openshift-csi-operator-master-e2e-openstack-cinder-csi

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.

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. rehearsals-ack Signifies that rehearsal jobs have been acknowledged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants