support non-kcp environment #8
Conversation
| return ctrl.Result{}, fmt.Errorf("there should be listed only one configmap with the given name '%s' for the given namespace '%s' when the clusterName is not available", cm.Name, cm.Namespace) | ||
| } | ||
| found = true | ||
| log.Info("Found in listed configmaps", "namespace", cm.Namespace, "name", cm.Name) |
There was a problem hiding this comment.
I'd move this back out (just inside the else block) and change it back to the List: got... log message. There can be multiple configmaps in a cluster across all namespaces, and this is supposed to show all of them.
There was a problem hiding this comment.
Yes, but imagine what it looks like when it's "connected" to a normal OpenShift cluster - there are approx. 550 ConfigMaps in the cluster and all of them are printed every time when the leader election updates its own configmap. This makes the whole log unreadable.
That's why I change it so it prints the count of all CMs found either in the cluster or in the workspace, plus:
- when it's running against kcp cluster (clusterName is set) then it shows all of them (as you requested)
- when it's running against a normal cluster, then it only finds the right ConfigMap which triggered the reconcile, and prints only that one
go.mod
Outdated
| @@ -77,6 +77,7 @@ require ( | |||
| ) | |||
|
|
|||
| replace ( | |||
| github.com/kcp-dev/apimachinery => github.com/MatousJobanek/apimachinery v0.0.0-20220615142058-8ebeb03b4114 | |||
There was a problem hiding this comment.
Yeah, I added it because it depends on the changes in my fork - so in case someone would want to try/test it. My plan was to remove it before merging the PR ;-)
removed in b3b512e
main.go
Outdated
|
|
||
| setupLog.Info("Using virtual workspace URL", "url", cfg.Host) | ||
|
|
||
| mgr, err = kcp.NewClusterAwareManager(cfg, ctrl.Options{ |
There was a problem hiding this comment.
Pull ctrl.Options out to a variable so it's not duplicated?
There was a problem hiding this comment.
I made that change, but somehow forgot to push it 🤦♂️
changed in a93e0ba
main.go
Outdated
| @@ -175,3 +212,27 @@ func restConfigForAPIExport(ctx context.Context, cfg *rest.Config, apiExportName | |||
|
|
|||
| return cfg, nil | |||
| } | |||
|
|
|||
| func apisKcpGroupPresent(restConfig *rest.Config) bool { | |||
There was a problem hiding this comment.
(no action required) I'd probably call this kcpAPIsGroupPresent
There was a problem hiding this comment.
renamed 77c8d19
|
Thanks a lot for your review @ncdc |
|
Updated the github.com/kcp-dev/apimachinery dependency in faa7b15 to use the merged latest changes |
This improves the code to support also non-kcp environment.
it depends on:
the changes introduced in this PR:
default-crdthat includes the CRDs into the manifests to be deployeddeploy-crdto deploy thatdefault-crdoverlayAPIExportand its contentnamespaces,secrets, andconfigmapsConfigMapReconcilerso it doesn't list allconfigmapsif it runs in a normal clustermain.goso it:apis.kcp.dev groupis present - if not, then it creates the manager using standard cache & client--api-export-namefixes: kcp-dev/apimachinery#32