-
Notifications
You must be signed in to change notification settings - Fork 916
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
karmadactl: add image pull secret flags for karmadactl init #3237
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #3237 +/- ##
=======================================
Coverage 49.20% 49.21%
=======================================
Files 203 203
Lines 18354 18372 +18
=======================================
+ Hits 9032 9041 +9
- Misses 8835 8843 +8
- Partials 487 488 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Hi @my-git9 Can you explain more about why we need this flag in the PR description? |
The description has been updated, thanks |
It makes sense to me. I guess it should be used along with |
The |
I'd like to invite @lonelyCZ for comments. |
I think it seems rare for using
We should only consider a common case, otherwise it even need to set different |
Do you mean the kubelet knows if the specified registry requires a secret? For public registry, the |
kubelet will try to select the secret by matching the registry. If not match, it will try without secrets. |
Yes, I just tried it that was ok. |
Thanks, I get it. Then, we don't need two flags for the pull secrets. |
48f7af8
to
f217d22
Compare
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.
Please share a test report here.
I wonder how to use it, especially how to prepare the secrets.
If we want to use this parameter, we should have the following steps:
The result:
|
OK, I get it. I guess we can explain it in the flag usage. |
f217d22
to
2d2b053
Compare
Updated, thanks |
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.
Generally looks good to me.
Please @lonelyCZ take a look.
And consider if we need to add validation for checking the existence of the secret.
2d2b053
to
37aca6f
Compare
37aca6f
to
a26c3dd
Compare
a26c3dd
to
97306d6
Compare
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.
Generally looks good to me now.
/assign @lonelyCZ
Signed-off-by: xin.li <xin.li@daocloud.io>
97306d6
to
0065f18
Compare
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.
/lgtm
/assign @lonelyCZ
Thanks.
I just tested it in my env that worked fine! /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lonelyCZ 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 |
What type of PR is this?
/kind feature
What this PR does / why we need it:
In a production environment, registry are generally non-public. If the registry we get from the administrator is private, we need to configure the imagepullsecret in the application's yaml file.
So I think it would be better if
karmadactl init
could specify imagepullsecret.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: