Skip to content

Pushes internal arm64 CentOS image and adds RATIONALE.md about Travis#376

Merged
codefromthecrypt merged 3 commits intomasterfrom
rationale-arm64
Sep 27, 2021
Merged

Pushes internal arm64 CentOS image and adds RATIONALE.md about Travis#376
codefromthecrypt merged 3 commits intomasterfrom
rationale-arm64

Conversation

@codefromthecrypt
Copy link
Copy Markdown
Contributor

A lot of thought went into the decision of whether or not to use
emulation for arm64. However, this was spread out across discussions and
PRs, but not in the codebase. This adds a RATIONALE section with the
background, which is that this is temporary until GHA supports arm64.

Note, the same would apply if we start supporting s390x, except we don't
at the moment, which is why that isn't mentioned. s390x support would
need to happen first in Envoy.

Obviates #373

@codefromthecrypt codefromthecrypt force-pushed the rationale-arm64 branch 2 times, most recently from 3897e62 to 3b46525 Compare September 24, 2021 20:19
A lot of thought went into the decision of whether or not to use
emulation for arm64. However, this was spread out across discussions and
PRs, but not in the codebase. This adds a RATIONALE section with the
background, which is that this is temporary until GHA supports arm64.

Note, the same would apply if we start supporting s390x, except we don't
at the moment, which is why that isn't mentioned. s390x support would
need to happen first in Envoy.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt codefromthecrypt changed the title Adds RATIONALE.md with common background about use of Travis for arm64 Pushes internal arm64 CentOS image and adds RATIONALE.md about Travis Sep 24, 2021
Signed-off-by: Adrian Cole <adrian@tetrate.io>
Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Copy Markdown
Contributor Author

tested with another github repo, pushing and consuming the image in the same way as we do here. Actually having travis use the other image would be a separate change.

@codefromthecrypt
Copy link
Copy Markdown
Contributor Author

Note: while I could list more reasons to not default to emulation, such as having to explain convoluted workarounds to actions that don't support it, I think there are enough reasons listed in the rationale for now. If we need more, definitely there's an advantage to being able to not do or explain this:

      # We do re-tagging for easier usage when testing using emulation, so we can forcefuly pull
      # arm64 image by directly specifiying the tag without the need of supplying
      # `--platform linux/arm64`. Using `uses: docker://` only allows to override `args`, but not
      # the `--platform` flag.
      # Reference: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-using-the-github-packages-container-registry
      # Related: https://github.com/actions/virtual-environments/issues/1368
      - name : Re-tag for arm64 only
        uses: docker/build-push-action@v2
        with:
          context: .
          platforms: linux/arm64 # Build and push for arm64 architecture only.
          tags:  ghcr.io/${{ github.repository_owner }}/func-e-internal:${{ matrix.target-tag }}-arm64

@codefromthecrypt
Copy link
Copy Markdown
Contributor Author

Another reason I didn't mention explicitly is the doubt emulation causes about the real reason why something fails. For example, in zipkin emulation bugs resulted in s390x to not work regardless of if it did in native arch. The complexity of layering several tools to achieve an arch means you have to consider in the case of func-e.. is this failing because envoy? or envoy over docker? or envoy over docker with emulation? Reducing the amount of things to wonder about when things fail is what you get when you move from complex -> complicated. I'll choose complicated (2 builds, one you barely touch) vs complex (config compat bomb waiting and only testable in a github actions run)

docker push ghcr.io/tetratelabs/func-e-internal:${{ matrix.target_tag }}

- name: Build and push
uses: docker/build-push-action@v2
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ps this is more opaque than I'd like as it is really hard to test what this might do locally (presumably buildx commands). Meanwhile we can use the action until it proves itself a nuisance (which might not happen ever if we are lucky!)

@codefromthecrypt codefromthecrypt merged commit cd493ea into master Sep 27, 2021
@codefromthecrypt codefromthecrypt deleted the rationale-arm64 branch September 27, 2021 00:43
@codefromthecrypt
Copy link
Copy Markdown
Contributor Author

I will kick off the workflow manually and deal with any fallout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants