-
Notifications
You must be signed in to change notification settings - Fork 42
feat: externalClusterReference with default naming scheme #215
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
base: master
Are you sure you want to change the base?
Changes from 10 commits
c8a450f
9e856da
8b4aaad
acb4cc9
55c0717
d50d28a
2ce99fa
aec406b
2919877
02c2c12
9aaffb4
67688df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,20 +9,21 @@ import ( | |
| "net" | ||
| "strings" | ||
|
|
||
| kcpv1alpha1 "github.com/clastix/cluster-api-control-plane-provider-kamaji/api/v1alpha1" | ||
| "github.com/clastix/cluster-api-control-plane-provider-kamaji/pkg/externalclusterreference" | ||
| kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1" | ||
| "github.com/pkg/errors" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| "k8s.io/client-go/util/retry" | ||
| "k8s.io/utils/ptr" | ||
| capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" | ||
|
|
||
| kcpv1alpha1 "github.com/clastix/cluster-api-control-plane-provider-kamaji/api/v1alpha1" | ||
| "github.com/clastix/cluster-api-control-plane-provider-kamaji/pkg/externalclusterreference" | ||
| ) | ||
|
|
||
| var ErrUnsupportedCertificateSAN = errors.New("a certificate SAN must be made of host only with no port") | ||
| var ErrTCPCollision = errors.New("Collision on remote TenantControlPlanes") | ||
|
|
||
| //+kubebuilder:rbac:groups=kamaji.clastix.io,resources=tenantcontrolplanes,verbs=get;list;watch;create;update | ||
|
|
||
|
|
@@ -57,6 +58,23 @@ func (r *KamajiControlPlaneReconciler) createOrUpdateTenantControlPlane(ctx cont | |
|
|
||
| tcp.Labels = kcp.Labels | ||
|
|
||
| // Check the KCP-UID label to avoid collsions if 2 clusters with the same name | ||
| // use the same namespace with externalClusterReference | ||
| // if label is not present, it will be added | ||
| if isDelegatedExternally && kcp.Spec.Deployment.ExternalClusterReference.DeploymentName != "" { | ||
| var tcpInCluster kamajiv1alpha1.TenantControlPlane | ||
| remoteClient.Get(ctx, types.NamespacedName{Namespace: tcp.Namespace, Name: tcp.Name}, &tcpInCluster) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason not to check the error here? |
||
|
|
||
| if val := tcpInCluster.Labels[kcpv1alpha1.KamajiControlPlaneUIDLabel]; val != "" { | ||
| if val != string(kcp.UID) { | ||
| return errors.Wrap(ErrTCPCollision, fmt.Sprintf("Collision on TenantControlPlane %s: Value of label '%s' does not match.", tcp.Name, kcpv1alpha1.KamajiControlPlaneUIDLabel)) | ||
| } | ||
| // label matches our kcp UID -> update TCP (nothing to do here) | ||
| } else { // label not present -> claim this TCP by adding it | ||
| tcp.Labels[kcpv1alpha1.KamajiControlPlaneUIDLabel] = string(kcp.UID) | ||
| } | ||
| } | ||
|
|
||
| if kubeconfigSecretKey := kcp.Annotations[kamajiv1alpha1.KubeconfigSecretKeyAnnotation]; kubeconfigSecretKey != "" { | ||
| tcp.Annotations[kamajiv1alpha1.KubeconfigSecretKeyAnnotation] = kubeconfigSecretKey | ||
| } else { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,23 @@ func (r *KamajiControlPlaneReconciler) handleDeletion(ctx context.Context, kcp v | |
| var tcp kamajiv1alpha1.TenantControlPlane | ||
| tcp.Name, tcp.Namespace = externalclusterreference.GenerateRemoteTenantControlPlaneNames(kcp) | ||
|
|
||
| // Check KamajiControlPlaneUIDLabel on TCP, to avoid deleting it if it doesn't belong to our KCP | ||
| if kcp.Spec.Deployment.ExternalClusterReference.DeploymentName != "" { | ||
|
|
||
| if err := remoteClient.Get(ctx, types.NamespacedName{Namespace: tcp.Namespace, Name: tcp.Name}, &tcp); err != nil { | ||
| if errors.IsNotFound(err) { | ||
| log.Info("resource may have been deleted") | ||
| } | ||
| log.Error(err, "unable to get remote TenantControlPlane") | ||
|
Comment on lines
+65
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case of Not Found error, we'll print the We should return |
||
| } | ||
|
|
||
| if val := tcp.Labels[v1alpha1.KamajiControlPlaneUIDLabel]; val != "" && val != string(kcp.UID) { | ||
|
||
| log.Info("Did not delete remote TenantControlPlane as it belongs to another KamajiControlPlane") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we log as argument the found and mismatching UID here? Just for the sake of debugging. |
||
|
|
||
| return nil | ||
| } | ||
| } | ||
|
|
||
| if tcpErr := remoteClient.Delete(ctx, &tcp); tcpErr != nil { | ||
| if errors.IsNotFound(tcpErr) { | ||
| log.Info("remote TenantControlPlane is already deleted") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this CEL rule is required when we have the ExternalClusterReference.DeploymentName one?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I understand it: this rule prevents the value from being unset while the one on ExternalClusterReference.DeploymentName prevents it from being modified
I based the config on this doc: https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/#immutablility-after-first-modification