metal-stack / mini-lab

a small, virtual setup to locally run the metal-stack
MIT License
55 stars 13 forks source link

Use `which` instead of `command -v` in CI. #128

Closed Gerrit91 closed 1 year ago

Gerrit91 commented 1 year ago

For some reason the built-in command command does not work on our CI runners:

Cleanup artifacts of previous runs
kind delete cluster --name metal-control-plane
make: command: Command not found
Deleting cluster "metal-control-plane" ...
docker-compose down
time="2022-11-10T0[9](https://github.com/metal-stack/mini-lab/actions/runs/3435611575/jobs/5728171911#step:6:10):37:00Z" level=warning msg="The \"DEPLOYMENT_BASE_IMAGE_TAG\" variable is not set. Defaulting to a blank string."
time="2022-11-[10](https://github.com/metal-stack/mini-lab/actions/runs/3435611575/jobs/5728171911#step:6:11)T09:37:00Z" level=warning msg="The \"DEPLOYMENT_BASE_IMAGE_TAG\" variable is not set. Defaulting to a blank string."
time="2022-11-10T09:37:00Z" level=warning msg="The \"METALCTL_IMAGE_TAG\" variable is not set. Defaulting to a blank string."
Warning: No resource found to remove for project "mini-lab".
rm -f /home/github-runner/actions-runner/_work/mini-lab/mini-lab/.kubeconfig
make: command: Command not found
sudo  destroy --topo mini-lab.cumulus.yaml
make: command: Command not found
sudo: destroy: command not found
make: *** [Makefile:[12](https://github.com/metal-stack/mini-lab/actions/runs/3435611575/jobs/5728171911#step:6:13)2: ...

Looks like there is a different shell context which does not have the built-ins. Also other people seem to have this issue when you google the issue. :/

Gerrit91 commented 1 year ago

@fhaftmann Can you please check if using which does also work for you? Otherwise we need to think about something else that works for CI as well as your use-cases.

harmathy commented 1 year ago

For some reason the built-in command command does not work on our CI runners:

Judging from this answer on Stackoverflow it is caused by make. I also had a look into the code and there is list of commands, which have to pass through a shell. Since version 4.3 command is listed there, which should have been there from the beginning.

Gerrit91 commented 1 year ago

Sounds reasonable. I hope that which is also ok for you. Otherwise, I'll need to adapt the CI.

harmathy commented 1 year ago

In general the behavior of which is not well defined, but this case we're targeting Linux, therefore that's no problem.

fhaftmann commented 1 year ago

I am fine with using which here.

See https://unix.stackexchange.com/questions/85249/why-not-use-which-what-to-use-then why I nowadays avoid which.