[WIP] Fix cicd image build path for lxd-initializer image #248
Conversation
- Modify inti container image and parameters
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the CI/CD image build path for the lxd-initializer image and modifies initialization container parameters to improve deployment consistency and reliability.
- Consolidates image build variables and updates image tags from dev version to production version
- Replaces Ubuntu base image with custom lxd-initializer image in init container
- Updates deployment configuration to use consistent image pull policies and adds node affinity rules
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lxd-initializer/lxd-initializer-daemonset.yaml | Complete removal of standalone DaemonSet configuration |
| lxd-initializer/integration.md | Updates documentation to reference new template file location |
| controllers/templates/lxd_initializer_ds.yaml | Major refactoring of init container logic and image configuration |
| Makefile | Consolidates LXD image build variables and updates version tags |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| initContainers: | ||
| - name: ensure-lxd | ||
| image: ubuntu:22.04 | ||
| image: us-east1-docker.pkg.dev/spectro-images/cluster-api/lxd-initializer:v0.6.1-spectro-4.7.13 |
There was a problem hiding this comment.
The image tag is hardcoded with a specific version. Consider using a variable or template substitution to make this configurable and consistent with the build system.
| image: us-east1-docker.pkg.dev/spectro-images/cluster-api/lxd-initializer:v0.6.1-spectro-4.7.13 | |
| image: {{ .Values.image.repository }}:{{ .Values.image.tag }} |
| nsenter -t 1 -m -p -- sh -c 'command -v apt-get >/dev/null 2>&1 && apt-get update || true' | ||
| nsenter -t 1 -m -p -- sh -c 'command -v apt-get >/dev/null 2>&1 && apt-get install -y snapd systemd || true' |
There was a problem hiding this comment.
The repeated nsenter -t 1 -m -p -- sh -c 'command -v apt-get >/dev/null 2>&1 && pattern creates code duplication. Consider extracting this into a shell function or combining the commands into a single nsenter call.
| nsenter -t 1 -m -p -- sh -c 'command -v apt-get >/dev/null 2>&1 && apt-get update || true' | |
| nsenter -t 1 -m -p -- sh -c 'command -v apt-get >/dev/null 2>&1 && apt-get install -y snapd systemd || true' | |
| nsenter -t 1 -m -p -- sh -c 'if command -v apt-get >/dev/null 2>&1; then apt-get update && apt-get install -y snapd systemd; fi || true' |
| CONTROLLER_IMG ?= ${REGISTRY}/${IMAGE_NAME} | ||
|
|
||
|
|
||
| LXD_IMAGE_NAME ?= "lxd-initializer" |
There was a problem hiding this comment.
The variable name should not have quotes around the string value. In Makefiles, quotes are literal and will be included in the variable value, which could cause issues when the variable is used in commands.
| LXD_IMAGE_NAME ?= "lxd-initializer" | |
| LXD_IMAGE_NAME ?= lxd-initializer |
AmitSahastra
changed the title
2 participants
This PR fixes the CI/CD image build path for the lxd-initializer image and modifies initialization container parameters to improve deployment consistency and reliability.