feat - object etcd migration #51
Conversation
fc2bda0 to
1c322be
Compare
Signed-off-by: Hélia Barroso <helia_barroso@hotmail.com>
|
@simonpasquier can you help review this? I'm not sure about this change. |
simonpasquier
left a comment
There was a problem hiding this comment.
Thanks for doing this! I had a look at the cert-manager CLI and they have a similar functionality. I wonder if we couldn't import/copy their logic?
https://github.com/cert-manager/cmctl/blob/main/pkg/upgrade/migrateapiversion/migrator.go
| ) | ||
|
|
||
| func MigrateCRDs(ctx context.Context, clientSets *k8sutil.ClientSets) error { | ||
| crds, err := clientSets.APIExtensionsClient.ApiextensionsV1().CustomResourceDefinitions().List(ctx, metav1.ListOptions{}) |
There was a problem hiding this comment.
what if we hardocde the list of CRDs supported by the tool? E.g. for now we only have alertmanagerconfigs (+ scrapeconfigs eventually).
| return fmt.Errorf("failed to list CRDs: %w", err) | ||
| } | ||
|
|
||
| for _, crd := range crds.Items { |
There was a problem hiding this comment.
(nit) To simplify the reading and error handling, I would loop over the CRDs to build the list of (CRD + desired version) and then migrate the result in another loop.
| for _, namespace := range namespaces.Items { | ||
| ns := namespace.Name | ||
|
|
||
| customResourcesInstances, err := clientSets.DClient.Resource(crdResourceVersion).Namespace(ns).List(ctx, metav1.ListOptions{}) |
There was a problem hiding this comment.
If you pass an empty namespace, I suppose that it will return resources for all namespace (not tested though).
| customResourcesInstances, err := clientSets.DClient.Resource(crdResourceVersion).Namespace(ns).List(ctx, metav1.ListOptions{}) | |
| customResourcesInstances, err := clientSets.DClient.Resource(crdResourceVersion).Namespace("").List(ctx, metav1.ListOptions{}) |
|
Also for context
|
Closes #26