-
Notifications
You must be signed in to change notification settings - Fork 2
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
Cluster #12
Cluster #12
Conversation
da292f7
to
d4dfe9d
Compare
3488dd8
to
1e7bd3e
Compare
Extract cluster ID from ConfigMap and the returned list of clusters.
|
||
id := fmt.Sprintf("/subscriptions/%s/resourcegroups/%s/providers/Microsoft.ContainerService/managedClusters/%s", | ||
keys["AZURE_SUBSCRIPTION_ID"], keys["AZURE_RESOURCE_GROUP"], | ||
keys["AZURE_RESOURCE_NAME"]) |
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.
Should we bail if some of the values here are missing? ... or maybe just log something?
Worst case we accidentally load the current cluster. Which is not that awful.
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.
Good question, this is designed so that if the ConfigMap doesn't exist, it will basically match an incomplete string, that is unlikely to match the ID that comes from the ProviderCluster.
func (p *AzureProvider) ClusterID(ctx context.Context, kubeClient client.Reader) (string, error) { | ||
configMap := &corev1.ConfigMap{} | ||
if err := kubeClient.Get(ctx, client.ObjectKey{Name: "extension-manager-config", Namespace: "kube-system"}, configMap); err != nil { | ||
return "", client.IgnoreNotFound(err) |
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.
Right.. if its missing we don't bail either. I'm not so stressed for now.
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.
Nice!
This adds support for determining a Cluster ID and getting the ID from the list of clusters.
This is then used to avoid reflecting the "current" cluster (the cluster the controller is running in) as a GitopsCluster which would lead to cluster duplication.