Skip to content
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.

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

OSASINFRA-3618: Add Cinder CSI support (part 2) #275

Merged
merged 19 commits into from
Oct 14, 2024

Conversation

stephenfin
Copy link
Contributor

@stephenfin stephenfin commented Sep 13, 2024

This is part 2 of the process of merging the OpenStack Cinder CSI Driver operator into csi-operator.

This PR addresses the fourth step in the migration guide, namely moving the operator to the new structure in csi-operator.

Follow-up PRs will remove the legacy/openstack-cinder-csi-driver-operator directory and update both the CI jobs and image builds to remove references to same.

You can compare the assets generated here using the following script, which relies on difftastic (available from the Fedora repos):

#!/usr/bin/env python3

import pathlib
import subprocess
import sys


assets = [
    p
    for p in pathlib.Path('openstack-cinder-csi-driver-operator/assets').glob(
        '*.yaml'
    )
]
assets += [
    p
    for p in pathlib.Path('openstack-cinder-csi-driver-operator/assets/rbac').glob('*.yaml')
]

csi_operator_assets = pathlib.Path(
    'csi-operator/assets/overlays/openstack-cinder/generated/standalone/'
)
for legacy_asset in assets:
    # map assets with different names
    asset_name = {
        'prometheus_rolebinding.yaml': 'prometheus_binding.yaml',
        'lease_leader_election_rolebinding.yaml': 'lease_leader_election_binding.yaml',
    }.get(legacy_asset.name, legacy_asset.name)
    csi_operator_asset = csi_operator_assets / legacy_asset.name
    if not csi_operator_asset.exists():
        print(f'ERROR: {csi_operator_asset} does not exist!')
        sys.exit(1)

    subprocess.run(
        f'difft --ignore-comments {legacy_asset} {csi_operator_asset}',
        shell=True,
    )

Previous steps:

@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 Sep 13, 2024
Copy link
Contributor

openshift-ci bot commented Sep 13, 2024

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

@gnufied
Copy link
Member

gnufied commented Sep 16, 2024

Did you check https://github.com/openshift/enhancements/blob/master/enhancements/storage/csi-driver-operator-merge.md for steps about how to merge operators?

Specifically the exact steps should be documented here - https://github.com/openshift/enhancements/blob/master/enhancements/storage/csi-driver-operator-merge.md#process-of-moving-operators-to-csi-operator-monorepo

But do let us know if we missed anything. It will be helpful.

@stephenfin stephenfin changed the title Add Cinder CSI support Add Cinder CSI support (part n) Sep 17, 2024
@stephenfin stephenfin changed the title Add Cinder CSI support (part n) Add Cinder CSI support (step n) Sep 17, 2024
@stephenfin stephenfin changed the title Add Cinder CSI support (step n) Add Cinder CSI support (part n) Sep 18, 2024
@stephenfin
Copy link
Contributor Author

stephenfin commented Sep 23, 2024

/retitle OSASINFRA-3618: Add Cinder CSI support (part 2)

@openshift-ci openshift-ci bot changed the title Add Cinder CSI support (part n) OSASINFRA-3618: Add Cinder CSI support (part 2) Sep 23, 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 23, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 23, 2024

@stephenfin: This pull request references OSASINFRA-3618 which is a valid jira issue.

In response to this:

This is work in progress as I still need to test this and iron out the last few kinks. I am trying to structure it such that it will be helpful HOWTO for others though.

Previous steps:

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-robot
Copy link

openshift-ci-robot commented Sep 23, 2024

@stephenfin: This pull request references OSASINFRA-3618 which is a valid jira issue.

In response to this:

This is part 2 of the process of merging the OpenStack Cinder CSI Driver operator into csi-operator.

This PR address the fourth step in the migration guide, namely moving the operator to the new structure in csi-operator.

Follow-up PRs will remove the legacy/openstack-cinder-csi-driver-operator directory and both the CI jobs and image builds to remove references to same.

Commits:

  • TODO

Previous steps:

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-robot
Copy link

openshift-ci-robot commented Sep 23, 2024

@stephenfin: This pull request references OSASINFRA-3618 which is a valid jira issue.

In response to this:

This is part 2 of the process of merging the OpenStack Cinder CSI Driver operator into csi-operator.

This PR address the fourth step in the migration guide, namely moving the operator to the new structure in csi-operator.

Follow-up PRs will remove the legacy/openstack-cinder-csi-driver-operator directory and both the CI jobs and image builds to remove references to same.

Previous steps:

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-robot
Copy link

openshift-ci-robot commented Sep 23, 2024

@stephenfin: This pull request references OSASINFRA-3618 which is a valid jira issue.

In response to this:

This is part 2 of the process of merging the OpenStack Cinder CSI Driver operator into csi-operator.

This PR address the fourth step in the migration guide, namely moving the operator to the new structure in csi-operator.

Follow-up PRs will remove the legacy/openstack-cinder-csi-driver-operator directory and both the CI jobs and image builds to remove references to same.

Previous steps:

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-robot
Copy link

openshift-ci-robot commented Sep 23, 2024

@stephenfin: This pull request references OSASINFRA-3618 which is a valid jira issue.

In response to this:

This is part 2 of the process of merging the OpenStack Cinder CSI Driver operator into csi-operator.

This PR address the fourth step in the migration guide, namely moving the operator to the new structure in csi-operator.

Follow-up PRs will remove the legacy/openstack-cinder-csi-driver-operator directory and both the CI jobs and image builds to remove references to same.

Previous steps:

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-robot
Copy link

openshift-ci-robot commented Sep 23, 2024

@stephenfin: This pull request references OSASINFRA-3618 which is a valid jira issue.

In response to this:

This is part 2 of the process of merging the OpenStack Cinder CSI Driver operator into csi-operator.

This PR address the fourth step in the migration guide, namely moving the operator to the new structure in csi-operator.

Follow-up PRs will remove the legacy/openstack-cinder-csi-driver-operator directory and update both the CI jobs and image builds to remove references to same.

Previous steps:

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 marked this pull request as ready for review September 23, 2024 09:57
@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 Sep 23, 2024
stephenfin added a commit to shiftstack/release that referenced this pull request Sep 23, 2024
The location of files have changed [1]. Reflect this.

[1] openshift/csi-operator#275

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
stephenfin added a commit to shiftstack/ocp-build-data that referenced this pull request Sep 23, 2024
This was removed in [1].

[1] openshift/csi-operator#275

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin
Copy link
Contributor Author

stephenfin commented Sep 23, 2024

Failures look real. I'm investigating.

Done. I'd forgotten to initialise an informer for the relevant namespace.

EDIT: ...and create the ClusterRoleBinding, since our controller currently needs to be privileged (due to use of host networking).

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

stephenfin commented Oct 9, 2024

@gnufied I have an ask: assuming there is nothing too egregious here, would you be okay with me addressing the comments here in a follow-up PR?

EDIT: In anticipation, I have proposed #292. It currently contains resolution to most of the comments here and I can address the remaining ones as necessary. Assuming this is okay, I'll rebase #292 once this merges.

EDIT 2: I've merged #292 back into this change after all.

Trivial nit.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This won't appear in the generated assets, but that's okay: this is most
relevant to OpenShift devs.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2024
@gnufied
Copy link
Member

gnufied commented Oct 9, 2024

/retest
/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2024
Copy link
Contributor

openshift-ci bot commented Oct 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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 Oct 9, 2024
Copy link
Contributor

openshift-ci bot commented Oct 9, 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/prow/security fcac8f2 link false /test security
ci/prow/e2e-azurestack-csi fcac8f2 link false /test e2e-azurestack-csi

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.

@stephenfin
Copy link
Contributor Author

/retest-required

@gnufied
Copy link
Member

gnufied commented Oct 14, 2024

/label px-approved
/label docs-approved

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Oct 14, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit e9b462f into openshift:master Oct 14, 2024
24 of 26 checks passed
@stephenfin stephenfin deleted the add-cinder-csi branch October 14, 2024 15:06
stephenfin added a commit to shiftstack/release that referenced this pull request Oct 14, 2024
The location of files have changed [1]. Reflect this.

[1] openshift/csi-operator#275

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: azure-file-csi-driver-operator
This PR has been included in build ose-azure-file-csi-driver-operator-container-v4.18.0-202410142041.p0.ge9b462f.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-aws-efs-csi-driver-operator
This PR has been included in build ose-aws-efs-csi-driver-operator-container-v4.18.0-202410142041.p0.ge9b462f.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: csi-driver-manila-operator
This PR has been included in build csi-driver-manila-operator-container-v4.18.0-202410142041.p0.ge9b462f.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-azure-disk-csi-driver-operator
This PR has been included in build ose-azure-disk-csi-driver-operator-container-v4.18.0-202410142041.p0.ge9b462f.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-aws-ebs-csi-driver-operator
This PR has been included in build ose-aws-ebs-csi-driver-operator-container-v4.18.0-202410142041.p0.ge9b462f.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-smb-csi-driver-operator
This PR has been included in build ose-smb-csi-driver-operator-container-v4.18.0-202410142041.p0.ge9b462f.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-openstack-cinder-csi-driver-operator
This PR has been included in build ose-openstack-cinder-csi-driver-operator-container-v4.18.0-202410142041.p0.ge9b462f.assembly.stream.el9.
All builds following this will include this PR.

openshift-merge-bot bot pushed a commit to openshift/release that referenced this pull request Oct 24, 2024
The location of files have changed [1]. Reflect this.

[1] openshift/csi-operator#275

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
ptalgulk01 pushed a commit to ptalgulk01/release that referenced this pull request Oct 29, 2024
…ift#57034)

The location of files have changed [1]. Reflect this.

[1] openshift/csi-operator#275

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
obochan-rh pushed a commit to obochan-rh/release that referenced this pull request Oct 31, 2024
…ift#57034)

The location of files have changed [1]. Reflect this.

[1] openshift/csi-operator#275

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
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. docs-approved Signifies that Docs has signed off on this PR 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. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants