travisghansen / argo-cd-helmfile

Integration between argo-cd and helmfile
MIT License
213 stars 55 forks source link

Issues related to sidecar CMP mode #37

Closed crabique closed 1 year ago

crabique commented 1 year ago

Hello, first of all I would like to thank you for this project, I find it very useful!

The problems I'm having are that upon the migration to use the CMP as a sidecar container, a few of applications on our cluster broke for mainly 2 reasons described below.

Missing bundled argo-cd tools

Some apps broke because of missing kustomize binary:

exec: "kustomize": executable file not found in $PATH COMMAND: kustomize build ...

Even though the binary is available in the main container of the pod.

I confirmed this with running something like this for all replicas of repo-server pod (-c helmfile-plugin):

wget https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize%2Fv5.0.1/kustomize_v5.0.1_linux_amd64.tar.gz
tar xfz kustomize_v5.0.1_linux_amd64.tar.gz
mv kustomize /home/argocd/krew/bin/ # (the only writeable directory in PATH)

After that, "hard refresh" on most broken applications fixed all the errors.

Of course I can add a shared-volume init-container to download kustomize, but this raises a few concerns about maintainability: argo-cd routinely updates their bundled tools using this config, so it has all of the tools out of the box with versions guaranteed to work well with argo-cd.

The image tagged as travisghansen/argo-cd-helmfile:v0.3.1 has helm version v3.11.1 which is older than what's in argo-cd master, but newer than in argo-cd latest (2.6.4), so this is a bit inconsistent and may cause difficult to debug issues in the future.

Maybe there is a way to somehow align with argo-cd tools version, e.g. refer to the tool-versions.sh file of the stable git tag? Or work with upstream to extend the copyutil init-container of the repo-server to also copy those tools to /run along with the cmp-server. Either way, your comment on this would be very valuable!

Permission denied for hacks/.../download.sh

The other issue is that some apps, for example kubevirt, require helmfile to execute a download script to get the manifests, and with the move to the sidecar those charts stopped working, I get the following error in argo-cd:

panic: unexpected error: fork/exec ../../hacks/kubevirt/download.sh: permission denied goroutine 98 [running]: github.com/helmfile/helmfile/pkg/helmexec.Output(0xc0009ee000, {0xc000ae7740, 0x1, 0x2?}) /home/runner/work/helmfile/helmfile/pkg/helmexec/runner.go:107 +0xced github.com/helmfile/helmfile/pkg/helmexec.ShellRunner.Execute({{0x2cc9508?, 0x40dfe7?}, 0xc000aa46d0?}, {0xc00011caa0?, 0xc000ae7701?}, {0xc0009a3840?, 0x40eb25?, 0x21eed20?}, 0xc000ca30b0?, 0x0) /home/runner/work/helmfile/helmfile/pkg/helmexec/runner.go:40 +0x12e github.com/helmfile/helmfile/pkg/event.(*Bus).Trigger(0xc000ae7bd0, {0x2588551, 0x7}, {0x0, 0x0}, 0x0?) /home/runner/work/helmfile/helmfile/pkg/event/bus.go:121 +0xa0b github.com/helmfile/helmfile/pkg/state.(*HelmState).triggerReleaseEvent(0x4ca43c?, {0x2588551, 0x7}, {0x0?, 0x0}, 0xc00096d818, {0x258a656, 0x8}) /home/runner/work/helmfile/helmfile/pkg/state/state.go:2312 +0x331 github.com/helmfile/helmfile/pkg/state.(*HelmState).triggerPrepareEvent(...) /home/runner/work/helmfile/helmfile/pkg/state/state.go:2275 github.com/helmfile/helmfile/pkg/state.(*HelmState).PrepareCharts.func2(0x441fc5?) /home/runner/work/helmfile/helmfile/pkg/state/state.go:1136 +0x17e github.com/helmfile/helmfile/pkg/state.(*HelmState).scatterGather.func1(0x0?) /home/runner/work/helmfile/helmfile/pkg/state/state_run.go:33 +0x26 created by github.com/helmfile/helmfile/pkg/state.(*HelmState).scatterGather /home/runner/work/helmfile/helmfile/pkg/state/state_run.go:32 +0x74

I did not dive very deep into this, but it looks to be some kind of noexec mount issue. Do you have any pointers on where to look to understand this better?

P.S.

Both of those are non-issues when using the configmap approach. Here is the relevant versions info:

travisghansen/argo-cd-helmfile:v0.3.1
ArgoCD: 2.6.5 (chart 5.26.2)

Thank you, and looking forward to your reply!

travisghansen commented 1 year ago

Welcome! I probably can’t align perfectly with argo-cd versions but I’m happy to add those tools to the image.

Let me check on the exec issue with someone from the argo team.

travisghansen commented 1 year ago

v0.3.2 has kustomize installed. Can you point me to the source of the kubevirt (or other chart) which execs? I've never seen that and so will try to run some tests.

crabique commented 1 year ago

Thank you, that is great!

As for the exec issue, it is a bit of a customized setup thing around helm hooks, here is how to reproduce it:

applications/kubevirt/helmfile.yaml

releases:
- name: kubevirt-operator
chart: manifests/v0.58.1/operator
hooks:
- events: ["prepare"]
showlogs: true
command: "../../hacks/kubevirt/download.sh"
args: ["v0.58.1", "operator"]
- name: kubevirt-cr
chart: manifests/v0.58.1/cr
needs:
- kubevirt-operator
hooks:
- events: ["prepare"]
showlogs: true
command: "../../hacks/kubevirt/download.sh"
args: ["v0.58.1", "cr"]

../../hacks/kubevirt/download.sh


#!/usr/bin/env bash

set -eo pipefail

KUBEVIRT_URL="https://github.com/kubevirt/kubevirt/releases/download" KUBEVIRT_VER="$1" KUBEVIRT_APP="$2"

[[ $KUBEVIRT_VER =~ ^v[0-9]{1}.[0-9]{1,3} ]] || (echo "Kubevirt version does not match regexp, for example v0.57.1"; exit 1) [[ $KUBEVIRT_APP =~ (operator|cr) ]] || (echo "Incorrect kubevirt app, expected operator or cr"; exit 1)

mkdir -p manifests/$KUBEVIRT_VER/$KUBEVIRT_APP

curl -Lk "$KUBEVIRT_URL/$KUBEVIRT_VER/kubevirt-$KUBEVIRT_APP.yaml" -o "manifests/$KUBEVIRT_VER/$KUBEVIRT_APP/manifest.yaml"

travisghansen commented 1 year ago

Can you exec into the container and find the tmp dir where the data is and see if the files are marked as executable? I’m wondering if something is different in how the data now gets sent to the sidecar which leaves the files without the executable biit.

crenshaw-dev commented 1 year ago

I’m wondering if something is different in how the data now gets sent to the sidecar which leaves the files without the executable biit.

That's exactly it. :-) https://github.com/argoproj/argo-cd/issues/9647

At the Argo CD contributors' meeting yesterday, Alex Matyushentsev said he plans to implement the "preserve executable" config option for the plugin.

For now, the plugin itself can be modified to chmod +x the file.

crabique commented 1 year ago

Ohh... Now I understand what's going on 🙂

For now, I changed it to command: "bash" and moved the script path to args[0], also changed the script to use wget instead of curl, so that nothing needs to be installed additionally. Not the most elegant workarounds but I expect the sidecar-cmp integration architecture will get better in the future, so this is completely fine.

Everything seems to be working perfectly now, thank you for your help!

travisghansen commented 1 year ago

I'll make sure wget/curl are in the images. Any other tools you think would be helpful?

crabique commented 1 year ago

Not at all, argo-cd containers don't have curl either, so previously we were adding it with an init-container, I just didn't get to see it's missing because of the -x error 🙂 Just wget is fine, as it's what is used in the Dockerfile, imo there is no need to bloat the image with something not also present in argo-cd images.

I hope in the future there will be some mechanism to pass down a PATH-able volume from argo-cd and not bundle duplicate stuff with every plugin, but for now I don't see anything obvious missing.

travisghansen commented 1 year ago

You can actually pass down a PATH yes.

Something like this is possible.

#  - name: KREW_ROOT
#    value: /home/argocd/.krew
  - name: PATH
    #value: $(KREW_ROOT)/bin:/opt/custom-tools:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
    value: /opt/custom-tools:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
crabique commented 1 year ago

Yes, I mean without downloading stuff, same way as argo-cd passes down /var/run/argocd/argocd-cmp-server, so argo-cd could pass down a directory like that with helm/kustomize/kubectl etc, and it can be added to volumeMounts and to PATH, so all of the binaries could be shared across argo-cd and all the CMP plugins.

travisghansen commented 1 year ago

You could create your own init image which has all binaries bundled and then just cp (instead of wget) to the custom tools dir.

travisghansen commented 1 year ago

When the new feature lands in argocd this should just work now https://github.com/travisghansen/argo-cd-helmfile/commit/e9951021760e4d6f51f172bb7fb39ef4a31e1dc1

crabique commented 1 year ago

Very nice, thanks for the quick fix!