-
Notifications
You must be signed in to change notification settings - Fork 759
[no-relnote] Fix prepare-release.sh script #1379
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| dependencies: | ||
| - name: node-feature-discovery | ||
| repository: https://kubernetes-sigs.github.io/node-feature-discovery/charts | ||
| version: 0.16.6 | ||
| digest: sha256:b125f183a60fb079c21a266b16b9d820853eba49b07405db2b909e6d2439f77d | ||
| generated: "2024-10-30T15:15:39.497349+01:00" | ||
| version: 0.17.3 | ||
| digest: sha256:6012a27c3109e9d29fb0729b38aa192a02c5ef177a971c5366449d13d9f092d0 | ||
| generated: "2025-08-20T18:56:48.301032+02:00" |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -130,9 +130,19 @@ pre_semver=${previous_version:1} | |||||
| # Modify files in the repo to point to new release | ||||||
| # | ||||||
| # Darwin or Linux | ||||||
| DOCKER="docker" | ||||||
| if [[ "$(uname)" == "Darwin" ]]; then | ||||||
| SED="$DOCKER run -i --rm -v $(PWD):$(PWD) -w $(PWD) alpine:latest sed" | ||||||
| # Try gsed first (GNU sed installed via Homebrew) | ||||||
| if command -v gsed &>/dev/null; then | ||||||
| SED="gsed" | ||||||
| # Fall back to Docker if gsed not available | ||||||
| elif docker version &>/dev/null; then | ||||||
| SED="docker run -i --rm -v $(PWD):$(PWD) -w $(PWD) alpine:latest sed" | ||||||
| else | ||||||
| echo "ERROR: Neither gsed nor Docker is available on macOS." | ||||||
| echo "Install GNU sed with: brew install gnu-sed" | ||||||
| echo "Or ensure Docker Desktop is running." | ||||||
| exit 1 | ||||||
| fi | ||||||
| else | ||||||
| SED="sed" | ||||||
| fi | ||||||
|
|
@@ -175,10 +185,16 @@ git commit -s -m "Bump version for $release release" | |||||
| echo Patching deployments to refer to $semver | ||||||
| find deployments/static -type f \( -name "*.yaml" -o -name "*.yml" -o -name "*.template" \) -type f ! -name "nfd.yaml" \ | ||||||
| -exec $SED -E -i \ | ||||||
| -e s",^([[:space:]]+)app.kubernetes.io\/version:.+$,\1app.kubernetes.io\/version: $semver," \ | ||||||
| -e s",^([[:space:]]+)- image:.+$,\1- image: $container_image," \ | ||||||
| -e s",^([[:space:]]*)app.kubernetes.io\/version:.+$,\1app.kubernetes.io\/version: $semver," \ | ||||||
|
||||||
| -e s",^([[:space:]]*)app.kubernetes.io\/version:.+$,\1app.kubernetes.io\/version: $semver," \ | |
| -e s",^([[:space:]]+)app.kubernetes.io\/version:.+$,\1app.kubernetes.io\/version: $semver," \ |
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.
@ArangoGutierrez was there a case that required this to be changed?
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 did a dry run, and the image: on the files at deployments/static were not edited.
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.
That doesn't really make sense since the output for the following (on main) are the same:
$ grep -R -E "^\s+-\s+image:" deployments/static
deployments/static/gpu-feature-discovery-daemonset.yaml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/nvidia-device-plugin-compat-with-cpumanager.yml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/gpu-feature-discovery-job.yaml.template: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/nvidia-device-plugin.yml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/gpu-feature-discovery-daemonset-with-mig-mixed.yaml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/gpu-feature-discovery-daemonset-with-mig-single.yaml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
and
$ grep -R -E "^\s*-\s+image:" deployments/static
deployments/static/gpu-feature-discovery-daemonset.yaml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/nvidia-device-plugin-compat-with-cpumanager.yml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/gpu-feature-discovery-job.yaml.template: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/nvidia-device-plugin.yml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/gpu-feature-discovery-daemonset-with-mig-mixed.yaml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
deployments/static/gpu-feature-discovery-daemonset-with-mig-single.yaml: - image: nvcr.io/nvidia/k8s-device-plugin:v0.17.1
Also looking at the diff here I don't see any changes where there are no leading spaces for the pattern that we're trying to match.
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: Running on main I see the following:
$ git diff --name-only upstream/main deployments/static | cat
deployments/static/gpu-feature-discovery-daemonset-with-mig-mixed.yaml
deployments/static/gpu-feature-discovery-daemonset-with-mig-single.yaml
deployments/static/gpu-feature-discovery-daemonset.yaml
deployments/static/gpu-feature-discovery-job.yaml.template
deployments/static/nvidia-device-plugin-compat-with-cpumanager.yml
deployments/static/nvidia-device-plugin.yml
Which -- with the exception of nfd.yaml -- seems to match the changes you have here.
When comparing it directly to your branch we see:
$ git diff --name-only static_deployments deployments/static | cat
deployments/static/nfd.yaml
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.
Umm I see, I'll review why on my hosts the script didn't worked as expected.
Copilot
AI
Aug 20, 2025
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.
Similar to the previous line, changing from [[:space:]]+ to [[:space:]]* could match lines that start directly with '- image:' without any indentation, potentially causing unintended replacements in YAML files where indentation is critical.
| -e s",^([[:space:]]*)- image:.+$,\1- image: $container_image," \ | |
| -e s",^([[:space:]]+)- image:.+$,\1- image: $container_image," \ |
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.
We should bump the nfd dependency as a separate PR. It is especially confusing that we happen to have the same version here.