-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[2.17] fix imageRepository path for CoreDNS v1.8.0 #8182 #8183
[2.17] fix imageRepository path for CoreDNS v1.8.0 #8182 #8183
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. I understand the commands that are listed here. |
Welcome @sebfere! |
Hi @sebfere. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sebfere 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 |
CLA done. |
I'll review this next week but I don't see why would this be needed as coredns 1.8.0 is set for a long time in master without any issue 🤔 |
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.
@MRoci Please take a look at this if possible.
@@ -21,5 +21,5 @@ etcd: | |||
{% endif %} | |||
dns: | |||
type: CoreDNS | |||
imageRepository: {{ coredns_image_repo | regex_replace('/coredns.*$','') }} | |||
imageRepository: {{ coredns_image_repo | regex_replace('/coredns$','') }} |
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.
The above has been updated since #7570 for coredns versions <=1.7.0.
It would be safer to have some condition for switching the above for versions.
/check-cla |
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.
with kubernetes/kubernetes@38a41e1 and kubernetes release 1.21.2
kubeadm's constant CoreDNSImageName
has been changed from coredns/coredns
to coredns
.
Because of that setting imageRepository
to k8s.gcr.io/coredns
to pull k8s.gcr.io/coredns/coredns:v1.8.0
makes sense to me, but from what i see this should be handled automatically by kubeadm
itself and this change should not be necessary
Anyway, this won't be merged as long as it's not done in master prior to release branch |
It's in the commit you mentioned: @sebfere |
for custom repositories for sure but as per #8182 this problem affects also users that have the default I think that it can be caused when trying to pull WIth this fix |
Doesn't Kupespray automatically update Kubeadm to the correct version of Kubeadm during deployment ?
CoreDNS <= 1.7 is already handled per the existing code, and I think Kubeadm
|
@kien-truong |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. In response to this:
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. |
Thanks to the fix, I can now deploy the Kubernetes cluster as kubeadm can download the coredns image.
Resulting
/etc/kubernetes/kubeadm-images.yaml
: