-
Notifications
You must be signed in to change notification settings - Fork 2
[node-manager] mcm-use-node-manager-bootstrap-token #15
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
Conversation
Co-authored-by: Artem Kulyabin <[email protected]>
3e9d9eb
to
b1e1d88
Compare
} | ||
|
||
secret = &secretListNodeManager.Items[0] |
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.
i think we need to sort by metadata.creationTimestamp
by desc and get first (get last created token)
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.
- Need to fallback to old logic if secrets not found
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 sort and old logic(create token by mcm if node manager tokens not found)
secret, err = c.targetCoreClient.CoreV1().Secrets(metav1.NamespaceSystem).Get(secretName, metav1.GetOptions{}) | ||
labelSelector := "module=node-manager" | ||
secretListNodeManager, err := c.targetCoreClient.CoreV1().Secrets(metav1.NamespaceSystem).List(metav1.ListOptions{ | ||
LabelSelector: labelSelector, |
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.
i think only label selector is not enough, because in the future we can add another secrets from node manager.
Need to filter by name prefix after getting by label
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.
- in the token we have label
node-manager.deckhouse.io/node-group=$GROUP_NAME
probably we need to get by this label depend on fileds from machine deployment or machine resource. Please think about 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.
add filter by label from node group name from machine labels
node-manager.deckhouse.io/node-group=$NG_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.
some nist and please fix CI
7ecebfc
to
b637bc7
Compare
85ad3b6
to
822ab1d
Compare
What this PR does / why we need it:
this mr allow to use node-manager boostrap token instead mcm token.
Which issue(s) this PR fixes:
resolve this issue
Special notes for your reviewer:
Release note: