This repository has been archived by the owner on Apr 22, 2020. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
👍
also, we should pass this flag ONLY if the kubeadm version is v1.8 or higher.
The versions below that didn't have this feature.
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.
Agreed. 👍
@shashidharatd I'm not familiar with kubeadm version management/check in kubernetes-anywhere. Could you please help fix this? 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.
Maybe can ues
KUBEADM_VERSION
, already have this variable in this file.But
KUBEADM_VERSION
is a link like :gs://kubernetes-release-dev/bazel/v1.10.0-alpha.0.631+e7ad6e60081887/bin/linux/amd64/
.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.
yes but the variable is also used in openstack and seems like there it uses "stable" version from another link. It makes it complicated to check the version. And I can't test it out on an gce cloud :(
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.
yep, its complicated. off hand what i could think of is to extract a
semver
from KUBEADM_VERSION and write asemver
matching function and based on the condition use the right option for--discovery-token-unsafe-skip-ca-verification
flag.AFAIK, the
KUBEADM_VERSION
can be either one of 2 values.stable
gs://
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.
@shashidharatd sounds great! Thanks! BTW, do you have cycle to help implement this?
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.
@xiangpengzhao, i am giving it a try. will raise a PR once my GCE local testing pass.
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.
@shashidharatd cool, thanks!