-
Notifications
You must be signed in to change notification settings - Fork 39.6k
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
[Federation] Remove unnecessary functions from develop.sh as part of … #38902
[Federation] Remove unnecessary functions from develop.sh as part of … #38902
Conversation
…deploy.sh deprecation. This is part of a big refactor to deprecate a short-lived deploy.sh mechanism that nobody really could use due to bugs.
@@ -123,7 +115,9 @@ function get_version() { | |||
kube_version="${KUBE_VERSION}" | |||
else | |||
# Read the version back from the versions file if no version is given. | |||
kube_version="$(jq -r '.KUBE_VERSION' ${VERSIONS_FILE})" | |||
kube_version="$(cat ${VERSIONS_FILE} | python -c '\ |
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 am not sure if this is the best way to do this.
Feel free to cc someone else who would know more about 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.
cc @ixdy.
This was a page taken straight out of @ixdy's book - kubernetes/release#207
|
||
# Update config after build and push, but before turning up the clusters | ||
# to ensure the config has the right image version tags. | ||
gen_or_update_config "${kube_version}" |
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.
Sorry why is this not required now?
It is deprecated but we dont want to intentionally break 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.
This function was only required to generate the config for deploying clusters, not the federation control plane. I am considering completely getting rid of the cluster deployment part.
To clarify this is a cleanup PR. Does not fix a specific bug/issue, right? |
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.
@nikhiljindal this is a bug fix. This fixes the test build failures we are having right now. This is a follow up to - PR #38888
|
||
# Update config after build and push, but before turning up the clusters | ||
# to ensure the config has the right image version tags. | ||
gen_or_update_config "${kube_version}" |
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.
This function was only required to generate the config for deploying clusters, not the federation control plane. I am considering completely getting rid of the cluster deployment part.
Thanks for the clarifications |
Automatic merge from submit-queue (batch tested with PRs 37468, 36546, 38713, 38902, 38614) |
This is part of a big refactor to deprecate a short-lived deploy.sh
mechanism that nobody really could use due to bugs.
cc @kubernetes/sig-federation-misc @irfanurrehman