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-3632: Add Hypershift support to openstack-cinder #525

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

stephenfin
Copy link
Contributor

@stephenfin stephenfin commented Oct 17, 2024

This is a continuation of #510, which set up the laid the groundwork for Hypershift integration by rejigging asset generation.

In this PR, we do the actual work of integrating Hypershift support. This is closely modeled on the azure-disk example, which should be no surprise given how similar the various CSI Driver Operators are.

Related changes

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

openshift-ci-robot commented Oct 17, 2024

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

In response to this:

This is a continuation of #510, which set up the laid the groundwork for Hypershift integration by rejigging asset generation.

In this PR, we do the actual work of integrating Hypershift support. This is closely modeled on the azure-disk example, which should be no surprise given how similar the various CSI Driver Operators are.

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.

Copy link
Contributor

openshift-ci bot commented Oct 17, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: stephenfin
Once this PR has been reviewed and has the lgtm label, please assign gnufied for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@stephenfin
Copy link
Contributor Author

/cc @EmilienM
/cc @mandre
/cc @MaysaMacedo

@stephenfin stephenfin changed the title OSASINFRA-3483: Add Hypershift support to openstack-cinder OSASINFRA-3632: Add Hypershift support to openstack-cinder Oct 17, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 17, 2024

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

In response to this:

This is a continuation of #510, which set up the laid the groundwork for Hypershift integration by rejigging asset generation.

In this PR, we do the actual work of integrating Hypershift support. This is closely modeled on the azure-disk example, which should be no surprise given how similar the various CSI Driver Operators are.

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
Contributor

openshift-ci-robot commented Oct 17, 2024

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

In response to this:

This is a continuation of #510, which set up the laid the groundwork for Hypershift integration by rejigging asset generation.

In this PR, we do the actual work of integrating Hypershift support. This is closely modeled on the azure-disk example, which should be no surprise given how similar the various CSI Driver Operators are.

Related changes:

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
Contributor

openshift-ci-robot commented Oct 17, 2024

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

In response to this:

This is a continuation of #510, which set up the laid the groundwork for Hypershift integration by rejigging asset generation.

In this PR, we do the actual work of integrating Hypershift support. This is closely modeled on the azure-disk example, which should be no surprise given how similar the various CSI Driver Operators are.

Related changes

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
Copy link
Contributor Author

/test e2e-vsphere-csi

@stephenfin stephenfin force-pushed the add-cinder-csi-part-2 branch 2 times, most recently from 2e832b0 to 9d919b0 Compare October 24, 2024 15:39
items:
- key: clouds.yaml
path: clouds.yaml
secretName: openstack-cloud-credentials
Copy link
Contributor

Choose a reason for hiding this comment

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

A CredentialsRequest needs to be added to the hosted cluster, otherwise no openstack-cloud-credentials will exist

Copy link
Contributor

Choose a reason for hiding this comment

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

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 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 Oct 29, 2024
@stephenfin stephenfin force-pushed the add-cinder-csi-part-2 branch 3 times, most recently from cbd0473 to 619b36c Compare October 30, 2024 17:22
This currently does zilch, but it establishes the framework we can use
as we work on Hypershift integration.

Signed-off-by: Stephen Finucane <[email protected]>
This (undocumented) annotation is used to indicate that the asset should
not be included in the mangement cluster. Why things are done this way
rather than simply duplicating the file in 'standalone' and
'hypershift/guest' is an interesting question, but I do not get paid
enough to ask such questions 😉

Signed-off-by: Stephen Finucane <[email protected]>
We do not want this to run in the management cluster in a HCP
deployment. This has no effect on the generated assets for the standlone
cluster.

Signed-off-by: Stephen Finucane <[email protected]>
We historically restricted the CSI Driver operator deployments to
master nodes. This obviously won't work for HCP deployments where the
hosted control plane runs on worker nodes.

Add patches to only add the relevant 'nodeSelector' and 'tolerations'
attributes in a standalone deployment scenario.

Signed-off-by: Stephen Finucane <[email protected]>
…ole to role

These permissions can be namespaced. They don't need to be cluster-wide.

Signed-off-by: Stephen Finucane <[email protected]>
These do not appear to be necessary and the conflict with the roles
granted by HCP's control-plane-operator [1].

[1] https://github.com/openshift/hypershift/blob/main/control-plane-operator/controllers/hostedcontrolplane/storage/assets/role.yaml

Signed-off-by: Stephen Finucane <[email protected]>
This is mostly inspired by azure-disk changes. Plenty of comments are
scattered through the code explaining what we're doing and why. Go read
those instead of this. Go on, go!

Signed-off-by: Stephen Finucane <[email protected]>
Generated with 'make update'. Nothing much to see here.

Signed-off-by: Stephen Finucane <[email protected]>
The final step in our journey: we start using the assets generated in
earlier commits and formally add support for Cinder on Hypershift.

Signed-off-by: Stephen Finucane <[email protected]>
@stephenfin stephenfin marked this pull request as draft October 31, 2024 14:45
@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 Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants