Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add namespaceOverride support for privateRegistry #1050

Merged
merged 1 commit into from Feb 10, 2022
Merged

add namespaceOverride support for privateRegistry #1050

merged 1 commit into from Feb 10, 2022

Conversation

life-
Copy link
Contributor

@life- life- commented Feb 9, 2022

Fix #940

@ks-ci-bot ks-ci-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 9, 2022
@24sama
Copy link
Collaborator

24sama commented Feb 10, 2022

Thanks for your contribution!

@24sama
Copy link
Collaborator

24sama commented Feb 10, 2022

Could you please update this example file? https://github.com/kubesphere/kubekey/blob/master/docs/config-example.md

@life-
Copy link
Contributor Author

life- commented Feb 10, 2022

Could you please update this example file? https://github.com/kubesphere/kubekey/blob/master/docs/config-example.md

ok

24sama
24sama requested changes Feb 10, 2022
View reviewed changes
apis/kubekey/v1alpha1/cluster_types.go Outdated
@@ -177,6 +177,7 @@ type RegistryConfig struct {
RegistryMirrors []string `yaml:"registryMirrors" json:"registryMirrors,omitempty"`
InsecureRegistries []string `yaml:"insecureRegistries" json:"insecureRegistries,omitempty"`
PrivateRegistry string `yaml:"privateRegistry" json:"privateRegistry,omitempty"`
NamespaceOverride string `yaml:"namespaceOverride" json:"namespaceOverride,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The v1alpha1 version of the struct does not require this field. You can remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

config/crd/bases/kubekey.kubesphere.io_clusters.yaml Outdated
@@ -255,6 +255,8 @@ spec:
type: array
privateRegistry:
type: string
namespaceOverride:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field also can be removed. The reason is the same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

manifests/deploy.yaml Outdated
@@ -184,6 +184,8 @@ spec:
type: array
privateRegistry:
type: string
namespaceOverride:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field also can be removed. The reason is the same as above. And I will update this deploy file later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@24sama
Copy link
Collaborator

24sama commented Feb 10, 2022 •
edited

Could you please update this example file? https://github.com/kubesphere/kubekey/blob/master/docs/config-example.md

ok

@life-
Thank you for the revision. And I add some review comments. Please take a look.

@life-
Copy link
Contributor Author

life- commented Feb 10, 2022 •
edited

@24sama Thanks for your code review, I have already done the change according to your review comments. Pls have a look.

@24sama
Copy link
Collaborator

24sama commented Feb 10, 2022

Hi @life-
And one last thing, could you please squash your commits as one commit? That will make our commits look clearer.

@life-
add namespaceOverride support for privateRegistry
be8cda6 
remove namespaceOverride in v1alpha1
@life-
Copy link
Contributor Author

life- commented Feb 10, 2022 •
edited

ok, I have squashed my commits as one commit. Thanks! @24sama

@24sama
Copy link
Collaborator

24sama commented Feb 10, 2022

Thanks @life-
/lgtm

@ks-ci-bot ks-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2022
@ks-ci-bot
Copy link
Collaborator

LGTM label has been added.

Git tree hash: 371ea2d4311b1837bc5e48f7785caddf7308f759

@24sama
Copy link
Collaborator

24sama commented Feb 10, 2022

/approve

@ks-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 24sama, life-

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ks-ci-bot ks-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 10, 2022
@ks-ci-bot ks-ci-bot merged commit ff3cde3 into kubesphere:master Feb 10, 2022
@24sama
Copy link
Collaborator

24sama commented Feb 15, 2022

/cherrypick release-2.0

@ks-ci-bot ks-ci-bot mentioned this pull request Feb 15, 2022
Merged
@ks-ci-bot
Copy link
Collaborator

@24sama: new pull request created: #1058

In response to this:

/cherrypick release-2.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@24sama 24sama 24sama requested changes

Assignees

@24sama 24sama

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add namespaceOverride param in registry configuration
3 participants
@life- @24sama @ks-ci-bot