Skip to content
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

whalesay does not support arm64 and should not be used as a workflow example #11858

Closed
2 of 3 tasks
erikschul opened this issue Sep 21, 2023 · 19 comments · Fixed by #13429
Closed
2 of 3 tasks

whalesay does not support arm64 and should not be used as a workflow example #11858

erikschul opened this issue Sep 21, 2023 · 19 comments · Fixed by #13429
Assignees
Labels
area/docs Incorrect, missing, or mistakes in docs area/upstream This is an issue with an upstream dependency, not Argo itself P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/bug

Comments

@erikschul
Copy link

erikschul commented Sep 21, 2023

Pre-requisites

  • I have double-checked my configuration
  • I can confirm the issues exists when I tested with :latest
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what you expected to happen?

Tag for arm64 doesn't exist:
https://hub.docker.com/r/docker/whalesay/tags

Error (exit code 64): failed to start command: fork/exec /usr/local/bin/cowsay: exec format error

Version

irrelevant

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

When following this guide:

https://argoproj.github.io/argo-workflows/quick-start/

which uses

https://raw.githubusercontent.com/argoproj/argo-workflows/master/examples/hello-world.yaml

which uses whalesay

Logs from the workflow controller

irrelevant

Logs from in your workflow's wait container

irrelevant
@terrytangyuan
Copy link
Member

I've been running whalesay on arm64 without issues.

@terrytangyuan
Copy link
Member

You are using cowsay based on your logs /usr/local/bin/cowsay: exec format error

@erikschul
Copy link
Author

erikschul commented Sep 21, 2023

@terrytangyuan It works with Docker on MacOS, but not with containerd on Kubernetes. I assume that Docker runs the container with x86 emulation.

The template uses image: docker/whalesay:latest.

A simple alternative is:

spec:
  entrypoint: echotest
  templates:
  - name: echotest
    container:
      image: alpine
      command: ["sh","-c"]
      args: ["echo","hello"]

@terrytangyuan
Copy link
Member

Would you like to submit a PR to update the examples?

@erikschul
Copy link
Author

It seems whalesay is used in 61 files in examples.
A different solution could be to provide an arm64 image somehow (asking Docker?).
Or to provide a notice in the docs "We use whalesay everywhere, which doesn't work on arm64."

@erikschul
Copy link
Author

But also, I experience this frequently. It typically takes me 5 minutes to realize that the problem is lack of arm64 support, but I'm getting used to it. I assume others will have the same experience, so I guess it's low priority. I'm happy to close the issue.
But it is a bit annoying when it happens, especially in a quick-start guide, where you have to figure out how to run the example with a different image.

@agilgur5 agilgur5 added area/docs Incorrect, missing, or mistakes in docs P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important labels Sep 21, 2023
@agilgur5
Copy link
Member

@erikschul as mentioned above, we'll happily take a PR to update the examples to a similar image that is cross-architecture compatible

@terrytangyuan
Copy link
Member

We can also use argosay image which is something we can manage https://github.com/argoproj/argo-workflows/tree/master/test/e2e/images/argosay/v2

@erikschul
Copy link
Author

Cool. I'll take a look when I'm more familiar with Argo Workflows.

@h0tbird
Copy link

h0tbird commented May 12, 2024

We can also use argosay image which is something we can manage https://github.com/argoproj/argo-workflows/tree/master/test/e2e/images/argosay/v2

Despite #10435, an argoproj/argosay arm64 image has not been published on Docker Hub.

@agilgur5 agilgur5 added the area/upstream This is an issue with an upstream dependency, not Argo itself label May 12, 2024
@agilgur5
Copy link
Member

For reference, I threw up a tweet at Docker to try and get their attention about the issues with whalesay, since it affects a lot more than Argo and not even just arm64 either: docker/whalesay#6, docker/whalesay#7.

Despite #10435, an argoproj/argosay arm64 image has not been published on Docker Hub.

Yea there's apparently no CI process for that and it was last manually pushed: #11613 (comment). I'd prefer to remove argosay entirely (as it's no longer needed and would reduce complexity) and rely on whalesay per #11613 (comment), but if Docker doesn't update whalesay, we may have to maintain argosay in the long-term

@agilgur5 agilgur5 added the solution/suggested A solution to the bug has been suggested. Someone needs to implement it. label Jul 23, 2024
@agilgur5
Copy link
Member

PRs welcome for a replacement with another minimal trusted image that can do a simple echo, such as busybox, alpine, etc.

@erikschul
Copy link
Author

What about
https://hub.docker.com/_/hello-world

@agilgur5
Copy link
Member

Docker Support mentioned that in docker/whalesay#6 (comment), but as I wrote there, it prints out Docker CLI usage instructions after the "hello", so that would be very confusing to see in an example.

@Joibel
Copy link
Member

Joibel commented Jul 23, 2024

Another option would be to resurrect argosay.

@agilgur5
Copy link
Member

agilgur5 commented Jul 23, 2024

argosay was mentioned above as well, and still has some unresolved problems. But as I wrote above, I'd prefer to remove argosay entirely anyway as I don't believe it should be necessary anymore with Emissary.

Reducing the code surface is definitely preferable, and busybox is more than good enough as a replacement. We already have other third-party images in examples and tests as well (like alpine, influxdb, etc etc)

@Joibel
Copy link
Member

Joibel commented Jul 25, 2024

Yeah, it's unnecessary, but we could get a cute octopus argonaut to say stuff 😄

Reducing things is better. Marking as a good first issue. Two PRs could be done:

  • Replacement of whalesay and argosay in examples & docs
  • Removal of legacy argosay from the codebase

@Joibel Joibel added the good first issue Good for newcomers label Jul 25, 2024
@agilgur5
Copy link
Member

agilgur5 commented Jul 25, 2024

Could also split the argosay replacement as a third PR (i.e. one for whalesay one for argosay).

I didn't mark it as a good first issue as whalesay alone is used ~191 times in the codebase (as mentioned in #13388 (comment)). It's also not a pure find and replace as you'd also have to change the command, template names + entrypoints, and any results output in docs. The sheer size and some required attention to further consequences felt like a bit much, especially compared to some of the lower quality first contributions we've had (that also don't respond to review comments with some frequency). I only marked it as solution/suggested as such

@agilgur5 agilgur5 self-assigned this Aug 2, 2024
@agilgur5 agilgur5 removed the good first issue Good for newcomers label Aug 2, 2024
@agilgur5
Copy link
Member

agilgur5 commented Aug 2, 2024

I'm going to tackle this since it is a very confusing issue to new users and has gotten a couple related issues too (on manifest v1, for instance). It also does have a lot of usage as I mentioned above so is not the simplest issue either, and no one's picked it up since it was marked as a good first issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Incorrect, missing, or mistakes in docs area/upstream This is an issue with an upstream dependency, not Argo itself P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants