Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

fix: Nutanix CSI credentials Secret creation #34

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

dkoshkin
Copy link
Collaborator

@dkoshkin dkoshkin commented Apr 5, 2024

Depends on dkoshkin/build-add-nutanix-csi-to-examples

The existing code created a ClusterResourceSet with the user provided Secret. However, that won't work unless that Secret has an embedded Secret in it. Keeping it simple and just creating the Secret using the remote client. This way we can bubble up errors to the Cluster object.


Used the example cluster files and was able to create a Pod with a PV (spec from konvoy e2e files)

kubectl get pv -A
NAME                                       CAPACITY   ACCESS MODES   RECLAIM POLICY   STATUS   CLAIM                                                                  STORAGECLASS     REASON   AGE
pvc-f0369ac2-ce64-49ce-a472-6e895b1f423a   10Gi       RWO            Delete           Bound    e2e-volume-pvc-bound-to-pv/pv-volume-volume-pvc-bound-to-pv-tester-0   nutanix-volume            10m

@dkoshkin dkoshkin self-assigned this Apr 5, 2024
@github-actions github-actions bot added the fix label Apr 5, 2024
@dkoshkin dkoshkin force-pushed the dkoshkin/fix-nutanix-csi branch 2 times, most recently from bab3b56 to ffeea76 Compare April 5, 2024 20:37
@github-actions github-actions bot added fix and removed fix labels Apr 5, 2024
@dkoshkin dkoshkin force-pushed the dkoshkin/build-add-nutanix-csi-to-examples branch from b9c569c to 6d09be8 Compare April 5, 2024 21:11
@faiq
Copy link

faiq commented Apr 8, 2024

However, that won't work unless that Secret has an embedded Secret in it.

Can you explain what this means? I don't think I quite understand. Can you show what a secret would look like?

@dkoshkin
Copy link
Collaborator Author

dkoshkin commented Apr 8, 2024

However, that won't work unless that Secret has an embedded Secret in it.

Can you explain what this means? I don't think I quite understand. Can you show what a secret would look like?

Yes @faiq , it would be something like this.

---
apiVersion: v1
kind: Secret
metadata:
  name: ${CLUSTER_NAME}-pe-creds-for-csi # this is the Secret that's on the bootstrap/management cluster
  labels:
    cluster.x-k8s.io/provider: nutanix
type: addons.cluster.x-k8s.io/resource-set
stringData:
  secret: |
    apiVersion: v1
    kind: Secret
      name: nutanix-csi-credentials # name that CSI driver expects
      namespace: ntnx-system # namespace of where the driver is installed
    stringData:
      key: ${NUTANIX_PRISM_ELEMENT_ENDPOINT}:${NUTANIX_PORT}:${NUTANIX_USER}:${NUTANIX_PASSWORD}

@faiq
Copy link

faiq commented Apr 8, 2024

We ended up going with the CRS approach with @jimmidyson recommendation

#7 (comment)

I think it makes sense to keep a consistent of delivering things to the workload cluster.

However I do like a lot of your other changes

@dkoshkin
Copy link
Collaborator Author

dkoshkin commented Apr 8, 2024

We ended up going with the CRS approach with @jimmidyson recommendation

#7 (comment)

I think it makes sense to keep a consistent of delivering things to the workload cluster.

However I do like a lot of your other changes

I guess we could do the same with this Secret and create an embedded Secret on user's behalf, but I would like to discuss the overall approach with you and @jimmidyson again please.

I'm not sure if we gain anything much with CRS for these, it ties us closer to CRS (even for addons that have a CAAPH strategy) and makes it more difficult to debug failures since we aren't able to push events to the Cluster object on failed CRS applies.

Base automatically changed from dkoshkin/build-add-nutanix-csi-to-examples to main April 9, 2024 15:29
@github-actions github-actions bot added fix and removed fix labels Apr 9, 2024
The existing code created a ClusterResourceSet with the user provided Secret.
However, that won't work unless that Secret has an embedded Secret in it.
Copy link

@faiq faiq left a comment

Choose a reason for hiding this comment

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

Great changes!

Copy link

@supershal supershal left a comment

Choose a reason for hiding this comment

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

LGTM. nice improvement. Cleanup of the secret should be handled by CAREN in delete cluster hook in addition to uninstalling helm release.

@dkoshkin dkoshkin merged commit 21f4055 into main Apr 9, 2024
16 checks passed
@dkoshkin dkoshkin deleted the dkoshkin/fix-nutanix-csi branch April 9, 2024 19:01
@github-actions github-actions bot mentioned this pull request Apr 9, 2024
jimmidyson pushed a commit that referenced this pull request Apr 11, 2024
The existing code created a ClusterResourceSet with the user provided Secret.
However, that won't work unless that Secret has an embedded Secret in it.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants