-
Notifications
You must be signed in to change notification settings - Fork 892
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
support to promote dependency resources automatically by using '-d=true' #1863
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
72eff5f
to
1b8f541
Compare
dd2d000
to
acc0861
Compare
/assign @lonelyCZ |
Thanks @duanmengkk , I will review it ASAP. Could you give a sample of workflow including |
acc0861
to
db38cb1
Compare
Signed-off-by: duanmeng <[email protected]>
95df015
to
71b44f4
Compare
Any problems with this PR? @lonelyCZ |
Thanks, I will review it ASAP. |
return promote(controlPlaneRestConfig, obj, gvr, opts) | ||
} | ||
|
||
func promoteDeps(obj *unstructured.Unstructured, opts CommandPromoteOption, mapper meta.RESTMapper, gvr schema.GroupVersionResource, controlPlaneRestConfig *rest.Config) error { | ||
interpreter := defaultinterpreter.NewDefaultInterpreter() |
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.
Is it limited if we only use defaultinterpreter
?
return promote(controlPlaneRestConfig, obj, gvr, opts) | ||
} | ||
|
||
func promoteDeps(obj *unstructured.Unstructured, opts CommandPromoteOption, mapper meta.RESTMapper, gvr schema.GroupVersionResource, controlPlaneRestConfig *rest.Config) error { | ||
interpreter := defaultinterpreter.NewDefaultInterpreter() |
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.
Is it limited if we only use defaultinterpreter
?
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.
What do you think ? should we support customizedInterpreter in promote?
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 it is necessary because we can promote any resource, especially support CRD/CR.
Perhaps, we can listen some suggestions form @RainbowMango @mrlihanbo @GitHubxsy
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 it is necessary because we can promote any resource, especially support CRD/CR.
Agree,But it looks a little difficult. As webhook handles resource in control plane,but promote
handles resources in member cluster.
Let me doing some research on 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.
I think it is necessary because we can promote any resource, especially support CRD/CR.
Agree,But it looks a little difficult. As webhook handles resource in control plane,but
promote
handles resources in member cluster. Let me doing some research on it.
Hey @duanmengkk I was looking into the issue could you give a little bit more context on the above comment you made. Thanks already
One question, when a CR is promoted, shall it's CRD to be promoted together? |
Perhaps, we should promote CRD firstly. |
Hi @duanmengkk , Is there any progress on this PR? |
No,We can close this pr first. I have no idea about it yet,unless we introduce a webhook to member cluster. |
Signed-off-by: duanmeng [email protected]
What type of PR is this?
support to promote dependency resources automatically by using '-d=true'
What this PR does / why we need it:
If users use promote resource from member cluster to control plane,he may need promote relevant resources automatically.
Which issue(s) this PR fixes:
Fixes #1862
Special notes for your reviewer:
Does this PR introduce a user-facing change?: