-
Notifications
You must be signed in to change notification settings - Fork 29
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
Use assisted installer admin kubeconfig if exists #81
Use assisted installer admin kubeconfig if exists #81
Conversation
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.
Looks good, just a minor comment.
return | ||
} | ||
if assistedInstallerAdminKubeconfig != nil { | ||
result, err = h.makeProfile(assistedInstallerAdminKubeconfig) |
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.
Maybe worth having some log here?
I.e. a debug/info log that indicates which kubeconfig is used.
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.
Makes sense, will do it.
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.
Added an info message for this, and also for the case where we use the registration kubeconfig.
e7e82e7
to
b9dd377
Compare
/lgtm |
|
||
// Make the profile data from the cluster admin kubeconfig: | ||
result, err = h.makeProfile(clusterAdminKubeconfig) | ||
return | ||
} | ||
|
||
// fetchAssistedInstallerAdminKubeconfig uses the given Kubernetes API client to fetch the admin | ||
// kubeconfig that is created by the assisted installer when cluster installation finishes. It | ||
// returns the serialized kubeconfig, or nil if there it doesn't exist, for example if the cluster |
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.
Typo: extra "there".
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.
Thanks Irina :-) , fixing it ...
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.
Done.
Currently the deployment manager service uses the admin kubeconfig that was used to register the cluster with ACM. But this will not exist if the cluster was created with assisted installer. To avoid that problem this patch changes the service so that it first check if the secret used by the assisted installer exists. If it does exist then it will use it. Signed-off-by: Juan Hernandez <[email protected]>
b9dd377
to
7d63419
Compare
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: irinamihai The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Currently the deployment manager service uses the admin kubeconfig that was used to register the cluster with ACM. But this will not exist if the cluster was created with assisted installer. To avoid that problem this patch changes the service so that it first check if the secret used by the assisted installer exists. If it does exist then it will use it.