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.

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

Build all Kubernetes versions available. Fix for https://github.com/flatcar/Flatcar/issues/1293 #36

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

jhaprins
Copy link

[Title: describe the change in one sentence]

Modify the workflow file to download information about available Kubernetes versions and build all raw images for the last maintenance release for every main release.

How to use

[ describe what reviewers need to do in order to validate this PR ]

Testing done

[Describe the testing you have done before submitting this PR. Please include both the commands you issued as well as the output you got.]

  • I created a fork of this project
  • I then did the changes needed for the workflow file to create desired outcome.
  • I pushed my changes to github to see if it would result in the images I want to have.


images=(
"kubernetes-v1.28.2"
$(curl https://raw.githubusercontent.com/kubernetes/website/main/data/releases/schedule.yaml | yq -r '.schedules[].previousPatches[0].release' | awk '{print "kubernetes-v"$1}')
Copy link
Member

@pothos pothos Jan 10, 2024

Choose a reason for hiding this comment

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

Thanks, we need to move the curl call into it's own line for error handling (i.e., K8S_VERS=$(curl …)), and also add some common curl options for robustness: -fsSL --retry-delay 1 --retry 60 --retry-connrefused --retry-max-time 60 --connect-timeout 20

I think we also need to filter out future targetDate entries, or?

@jhaprins
Copy link
Author

jhaprins commented Jan 10, 2024

I have done the following additional changes:

  • I moved the curl call outside of the array
  • I added the curl options you requested.
  • I check if the resulting string is not empty
  • If the string is empty I bail out with an error 1
  • I create the array of kubernetes versions
  • I add this array to the images array
  • The yq command already does select the correct versions.

Jan Hugo Prins

"docker-24.0.6"
"docker_compose-2.22.0"
"wasmtime-13.0.0"
)
images+=(${KBS_VERS_ARRAY[@]})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
images+=(${KBS_VERS_ARRAY[@]})
images+=("${KBS_VERS_ARRAY[@]}")

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. Will modify.

gawk

KBS_VERS=$(curl -fsSL --retry-delay 1 --retry 60 --retry-connrefused --retry-max-time 60 --connect-timeout 20 https://raw.githubusercontent.com/kubernetes/website/main/data/releases/schedule.yaml | yq -r '.schedules[].previousPatches[0].release' | awk '{print "kubernetes-v"$1}')
[[ -z "${KBS_VERS}" ]] && exit 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[[ -z "${KBS_VERS}" ]] && exit 1
if [[ -z "${KBS_VERS}" ]]; then; echo "Failed fetching Kubernetes versions"; exit 1; fi

Copy link
Author

Choose a reason for hiding this comment

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

Don't really like that if construct. I will make it a little different, but functional the same.
I hope you will agree.

Suggested change
[[ -z "${KBS_VERS}" ]] && exit 1
[[ -z "${KBS_VERS}" ]] && echo "Failed fetching Kubernetes versions" && exit 1

@pothos
Copy link
Member

pothos commented Jan 10, 2024

I think we also need to filter out future targetDate entries, or?

With that I meant that while the current entries have no future dates that might be the case and then the k8s version can't be downloaded because it's not released yet.

@jhaprins
Copy link
Author

With that I meant that while the current entries have no future dates that might be the case and then the k8s version can't be downloaded because it's not released yet.

No, this is not true.
Check for example 1.29.1, this one has a future date but is not included in my selection.
The jq selection rule fetches very specific previousPatches, and therefor it will not fail on next: patches which are not release yet. See: yq -r '.schedules[].previousPatches[0].release'

Do the curl in a seperate line and check if the string contains items.
Some better error handling.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants