Repository navigation
[kubernetes] Fixed k8s<1.32 creation - #2209
Merged
Merged
Conversation
Signed-off-by: Myasnikov Daniil <[email protected]>
Contributor
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
dosubot
Bot
added
size/M
Contributor
📝 WalkthroughWalkthroughThe pull request transitions from a single Kubernetes version build to a multi-version approach. The Makefile now reads versions from a YAML configuration file and iterates through multiple Kubernetes versions (1.30–1.35), generating per-version image tags and metadata files sequentially instead of maintaining a single unified build. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/apps/kubernetes/Makefile (1)
1-1: Updatehack/update-versions.shforKUBERNETES_VERSIONS.
make updatestill calls a script that rewritesKUBERNETES_VERSIONin this file, so after this change that branch only emits a warning. Cleaning up that stale step will keep the update workflow consistent withfiles/versions.yaml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/apps/kubernetes/Makefile` at line 1, The Makefile now derives KUBERNETES_VERSIONS from files/versions.yaml (KUBERNETES_VERSIONS = $(shell awk -F'"' '{print $$2}' files/versions.yaml)) but the update workflow still runs hack/update-versions.sh which rewrites a stale KUBERNETES_VERSION in this file; update hack/update-versions.sh (invoked by the make update target) to stop modifying the Makefile (remove or change the branch that writes KUBERNETES_VERSION) so it either updates files/versions.yaml only or emits a consistent warning, ensuring no stale rewrite of KUBERNETES_VERSIONS occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/apps/kubernetes/Makefile`:
- Around line 20-23: The build currently uses --build-arg
KUBERNETES_VERSION=$(ver) which only carries the minor tag; change the Makefile
to also pass the exact mapped patch from versions.yaml (e.g., add --build-arg
KUBERNETES_FULL_VERSION=$(call settag,$(ver)-$(TAG)) or a new variable that
resolves the exact patch) so the image build is pinned to the same patch applied
by update-k8s.sh; update the Dockerfile to accept ARG KUBERNETES_FULL_VERSION
and use that value when installing/pinning kubelet and kubeadm package versions
instead of the minor-only KUBERNETES_VERSION.
- Around line 18-30: The foreach loop over KUBERNETES_VERSIONS masks per-version
failures because iterations are separated with ';' so a failed build can be
followed by successful ones and the target exits 0; update the loop body in the
Makefile so each iteration chain uses '&&' instead of ';' (affecting the docker
buildx build, echo and rm steps for images/ubuntu-container-disk and settag
usage) and append '&& true' after the closing ')' of the foreach to ensure the
overall make recipe correctly propagates any per-version failures.
---
Nitpick comments:
In `@packages/apps/kubernetes/Makefile`:
- Line 1: The Makefile now derives KUBERNETES_VERSIONS from files/versions.yaml
(KUBERNETES_VERSIONS = $(shell awk -F'"' '{print $$2}' files/versions.yaml)) but
the update workflow still runs hack/update-versions.sh which rewrites a stale
KUBERNETES_VERSION in this file; update hack/update-versions.sh (invoked by the
make update target) to stop modifying the Makefile (remove or change the branch
that writes KUBERNETES_VERSION) so it either updates files/versions.yaml only or
emits a consistent warning, ensuring no stale rewrite of KUBERNETES_VERSIONS
occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33c3aa93-796b-437b-8ef1-6ae7c7b7ec28
📒 Files selected for processing (9)
packages/apps/kubernetes/Makefilepackages/apps/kubernetes/images/ubuntu-container-disk-v1.30.tagpackages/apps/kubernetes/images/ubuntu-container-disk-v1.31.tagpackages/apps/kubernetes/images/ubuntu-container-disk-v1.32.tagpackages/apps/kubernetes/images/ubuntu-container-disk-v1.33.tagpackages/apps/kubernetes/images/ubuntu-container-disk-v1.34.tagpackages/apps/kubernetes/images/ubuntu-container-disk-v1.35.tagpackages/apps/kubernetes/images/ubuntu-container-disk.tagpackages/apps/kubernetes/templates/cluster.yaml
💤 Files with no reviewable changes (1)
- packages/apps/kubernetes/images/ubuntu-container-disk.tag
Comment on lines
+18
to
+30
| $(foreach ver,$(KUBERNETES_VERSIONS), \ | ||
| docker buildx build images/ubuntu-container-disk \ | ||
| --build-arg KUBERNETES_VERSION=$(ver) \ | ||
| --tag $(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)) \ | ||
| --tag $(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)-$(TAG)) \ | ||
| --cache-from type=registry,ref=$(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)) \ | ||
| --cache-to type=inline \ | ||
| --metadata-file images/ubuntu-container-disk-$(ver).json \ | ||
| $(BUILDX_ARGS) && \ | ||
| echo "$(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver))@$$(yq e '."containerimage.digest"' images/ubuntu-container-disk-$(ver).json -o json -r)" \ | ||
| > images/ubuntu-container-disk-$(ver).tag && \ | ||
| rm -f images/ubuntu-container-disk-$(ver).json; \ | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== image-ubuntu-container-disk recipe =="
sed -n '17,30p' packages/apps/kubernetes/Makefile
echo
echo "== Shell behavior with ';' after a failed && chain =="
bash -lc 'false && echo skipped && echo skipped; true'
echo "exit=$?"Repository: cozystack/cozystack
Length of output: 1053
Fix error handling in foreach loop to prevent masking per-version build failures.
The ; terminator at the end of each iteration breaks the error chain. If any version's build fails, the loop continues to the next version, and the target exits 0 if the last iteration succeeds—even though earlier versions failed partially. Change the terminator from ; to && and add true after the closing paren to preserve error propagation across all iterations.
🛠️ Suggested change
image-ubuntu-container-disk:
$(foreach ver,$(KUBERNETES_VERSIONS), \
docker buildx build images/ubuntu-container-disk \
--build-arg KUBERNETES_VERSION=$(ver) \
--tag $(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)) \
--tag $(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)-$(TAG)) \
--cache-from type=registry,ref=$(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)) \
--cache-to type=inline \
--metadata-file images/ubuntu-container-disk-$(ver).json \
$(BUILDX_ARGS) && \
echo "$(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver))@$$(yq e '."containerimage.digest"' images/ubuntu-container-disk-$(ver).json -o json -r)" \
> images/ubuntu-container-disk-$(ver).tag && \
- rm -f images/ubuntu-container-disk-$(ver).json; \
- )
+ rm -f images/ubuntu-container-disk-$(ver).json && \
+ ) true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/apps/kubernetes/Makefile` around lines 18 - 30, The foreach loop
over KUBERNETES_VERSIONS masks per-version failures because iterations are
separated with ';' so a failed build can be followed by successful ones and the
target exits 0; update the loop body in the Makefile so each iteration chain
uses '&&' instead of ';' (affecting the docker buildx build, echo and rm steps
for images/ubuntu-container-disk and settag usage) and append '&& true' after
the closing ')' of the foreach to ensure the overall make recipe correctly
propagates any per-version failures.
Comment on lines
+20
to
+23
| --build-arg KUBERNETES_VERSION=$(ver) \ | ||
| --tag $(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)) \ | ||
| --tag $(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)-$(TAG)) \ | ||
| --cache-from type=registry,ref=$(REGISTRY)/ubuntu-container-disk:$(call settag,$(ver)) \ |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Minor -> patch mapping =="
sed -n '1,20p' packages/apps/kubernetes/files/versions.yaml
echo
echo "== Build args passed from the Makefile =="
rg -n -C2 'image-ubuntu-container-disk|--build-arg KUBERNETES_VERSION|metadata-file images/ubuntu-container-disk' packages/apps/kubernetes/Makefile
echo
echo "== Kubernetes-related ARGs/usages in the Dockerfile =="
rg -n -C2 '^(ARG|ENV)\s+KUBERNETES|KUBERNETES_VERSION|kubelet|kubeadm' packages/apps/kubernetes/images/ubuntu-container-disk/Dockerfile
echo
echo "== Runtime patch pin =="
rg -n -C1 'KUBELET_VERSION=.*kubernetes\.versionMap' packages/apps/kubernetes/templates/cluster.yamlRepository: cozystack/cozystack
Length of output: 2572
Thread the exact patch version from versions.yaml into the image build.
The Makefile currently passes only the minor version (e.g., v1.30) to the Docker build, while the exact patch (e.g., v1.30.14) is applied later at runtime via update-k8s.sh. This disconnects the build from the intended patch level. Since versions.yaml intentionally caps patches to Kamaji's supported boundary, the image build should explicitly pin the exact patch to guarantee consistency between build-time and runtime versions.
Show current build invocation
--build-arg KUBERNETES_VERSION=$(ver) \
Pass the mapped exact patch to the Dockerfile (e.g., via a second build argument), and use it to pin kubelet/kubeadm versions in the package installation step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/apps/kubernetes/Makefile` around lines 20 - 23, The build currently
uses --build-arg KUBERNETES_VERSION=$(ver) which only carries the minor tag;
change the Makefile to also pass the exact mapped patch from versions.yaml
(e.g., add --build-arg KUBERNETES_FULL_VERSION=$(call settag,$(ver)-$(TAG)) or a
new variable that resolves the exact patch) so the image build is pinned to the
same patch applied by update-k8s.sh; update the Dockerfile to accept ARG
KUBERNETES_FULL_VERSION and use that value when installing/pinning kubelet and
kubeadm package versions instead of the minor-only KUBERNETES_VERSION.
3 of 10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What this PR does
This PR adds version specific ubuntu base images to fix errors when base image has new deb packages of kubeadm and kubelet installed, but at runtime it was downgraded by replacing just binaries. Now update by replacing binaries works as intended - latest patch version of minor version used.
Core issue was in kubeadm<1.32 expecting conntrack binary in its preflight checks but it was not found. It happened because kubelet deb package dropped conntrack dependency since 1.32 (actually it absent in 1.31.14 too).
So now status of supported tenant k8s versions is:
kubeadm initentirely.Conntrack preflight check ignoring is legit for tenant kubernetes clusters because until 1.32 it was used in kube-proxy but cozystack k8s approach doesn't use kube-proxy (replaced with cilium).
Issue with conntrack may be mitigated with only
ignorePreflightErrors, but I think proper base image build will help to avoid similar bugs in future.Release note
Summary by CodeRabbit
Release Notes