-
Notifications
You must be signed in to change notification settings - Fork 123
makefile: update deploy/delete to use Helm charts and adjust the readme #2843
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
base: main
Are you sure you want to change the base?
makefile: update deploy/delete to use Helm charts and adjust the readme #2843
Conversation
| ``` | ||
|
|
||
| - Apply the kustomize.yaml configuration that you modified earlier with: | ||
| This will deploy the latest code from main. |
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'm not sure if there is an important distinction to make here (probably not), but for releases I think we plan to pin the CAA (and other images) to a specific version, so if you are on a release tag then it might deploy the released version.
src/cloud-api-adaptor/Makefile
Outdated
| else \ | ||
| $(eval PROVIDER_SECRETS_FILE := install/charts/peerpods/providers/$(CLOUD_PROVIDER)-secrets.yaml) | ||
| @if [ ! -f "$(PROVIDER_SECRETS_FILE)" ]; then \ | ||
| echo "Error: secrets file not found at $(PROVIDER_SECRETS_FILE)"; \ |
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.
libvirt (and maybe docker) doesn't have a secrets file, so this shouldn't fail?
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 provide empty secret files (e.g. src/cloud-api-adaptor/install/charts/peerpods/providers/libvirt-secrets.yaml.template) so the dev could just copy (but not edit) the template, and the experience is the same to the other providers.
I'm just trying to avoid another libvirt/docker specific rule/checking. I'm in doubt, to be honest, but If you think it's important then I can change 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.
I would like to avoid specific provider rules/checks as well... But at the same time, having this failing for libvirt and docker is odd.
What if instead this make target enforces some specific values files, we just get them from the user? Users defines a list of values via env variables, and the make target just run with them...
HELM_VALUES_FILES="list here including the secrets"
make deploy
?
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.
It is weird indeed. At this point I want to keep make deploy simple. I think I will add the guard for docker/libvirt then it can be improved later.
src/cloud-api-adaptor/Makefile
Outdated
| echo "Installing webhook" ;\ | ||
| $(MAKE) -C ../webhook deploy ;\ | ||
| else \ | ||
| $(eval PROVIDER_SECRETS_FILE := install/charts/peerpods/providers/$(CLOUD_PROVIDER)-secrets.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.
we don't commit any secrets. We commit templates, users/devs needs to copy the template and populate with their own secrets.
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.
So maybe on the previous commit at README, update with the instructions to copy the template and/or populate/set the secrets?
Replace operator/kustomize installation instructions with Helm chart deployment. The readme now: - References Helm charts as the recommended installation method - Shows make deploy command for development - Points to PeerPods Helm Chart README for detailed instructions - Notes that kustomize-based installation is deprecated Assisted-by: Cursor Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
The `make deploy` and `make delete` targets now: - Use helm install/uninstall instead of kubectl apply/delete - Support WEBHOOK_ENABLED variable (default: true) to enable/disable webhook - Install cert-manager automatically if webhook is enabled - Use development mode (Option A) from Helm chart README - Support separate enable/disable of webhook and resourceCtrl components Assisted-by: Cursor Signed-off-by: Wainer dos Santos Moschetta <[email protected]>
1144055 to
bf2c45a
Compare
|
@stevenhorsman @beraldoleal I think I addressed all your comments. looks better now? |
The
make deploynow installs the peerpods helm chat. Implements theOption A: Development/Testing (secrets.mode: create)scenario described in install/charts/peerpods/README.md. Assume the dev is going to set the values and secrets properly.ps: I was attempted to remove the target completely, but in the end I left it because I feel it can be useful for quick deployment on libvirt and docker for development.
Example:
$ CLOUD_PROVIDER=libvirt make deploy helm install peerpods install/charts/peerpods \ -f install/charts/peerpods/providers/libvirt.yaml \ -f install/charts/peerpods/providers/libvirt-secrets.yaml \ --set webhook.enabled=true \ --set resourceCtrl.enabled=true \ --dependency-update \ -n confidential-containers-system \ --create-namespace NAME: peerpods LAST DEPLOYED: Tue Feb 10 17:45:25 2026 NAMESPACE: confidential-containers-system STATUS: deployed REVISION: 1 TEST SUITE: NoneExample (delete):
I had to adjust the install/README.md. Took that opportunity to add a comment saying that the kustomize install method is deprecated.