search

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.5k stars 1.3k forks source link

cloud-config secret move from original cluster to target cluster #3603

Closed jichenjc closed 4 years ago

jichenjc commented 4 years ago

User Story

when move from original cluster to target cluster, the secrets with suffix are defined here:

https://github.com/kubernetes-sigs/cluster-api/blob/master/util/secret/consts.go in openstack implementation, we created a cloud conf secret with -cloud-conf suffix so doesn't fit for above requirement thus it's not moved

is there any recommended way to such reuquirement? we have a few options 1) let user recreate a new one after move complete 2) let provider do some logic handle , maybe set ownerreference ?(which IMHO is hacky) 3) move action include that , maybe add contract such as -cloud-conf suffix will be moved as well

I hope to go option 3, any suggestions?

fabriziopandini commented 4 years ago

area/clusterctl

fabriziopandini commented 4 years ago

IMO clusterctl can be extended to move additional secrets.

What about using the clusterctl.cluster.x-k8s.io/move annotation that we are already using for force moving CRD types; if the secret has the annotation, force move it no matter of ownership/soft ownership

jichenjc commented 4 years ago

that's also good suggestion, if you agree I can add that to Secret

but I don't know whether it fit for other stuffs such as Configmap or any other resources... please help to comment

fabriziopandini commented 4 years ago

If we can manage to get this implemented in the Discovery process (objectGraph.objToNode && objectGraph.ownerToVirtualNode) this could be generic: any object with clusterctl.cluster.x-k8s.io/move gets forceMove=true

@wfernandes opinions ^^ ?

jichenjc commented 4 years ago

I think so as well, but not sure whether it will bring some side effect ...

jichenjc commented 4 years ago

/assign

wfernandes commented 4 years ago

@fabriziopandini That suggestion makes sense. @jichenjc For context you can look at this PR which adds the ClusterctlMoveLabelName: https://github.com/kubernetes-sigs/cluster-api/pull/3337 Regarding side effects, hopefully the tests will catch them 😉

jichenjc commented 4 years ago

@wfernandes thanks, I will take a look at the suggested PR and work on this , thanks :)