-
Notifications
You must be signed in to change notification settings - Fork 49
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
Fix machine implementation updating #789
Conversation
f071ec3
to
425d8fa
Compare
1afa11b
to
508bdc0
Compare
Signed-off-by: Alexey Makhov <amakhov@mirantis.com>
Signed-off-by: Alexey Makhov <amakhov@mirantis.com>
Signed-off-by: Alexey Makhov <amakhov@mirantis.com>
34cefdc
to
457a0c7
Compare
return nil, err | ||
} | ||
|
||
resourceType := strings.ToLower(machineFromTemplate.GetKind()) + "s" |
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.
wdyt about find the apiresource.Name using resource kind and rely on the crd plural?
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.
Changed it to more reliable way
RESTConfig *rest.Config | ||
Scheme *runtime.Scheme | ||
ClientSet *kubernetes.Clientset | ||
DynamicClient dynamic.Interface |
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.
do we need DynamicClient
?
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.
Not anymore. Removed it, thanks!
Signed-off-by: Alexey Makhov <amakhov@mirantis.com>
Merge and patch existing machine objects to avoid owner conflict and/or admission webhook issues