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

Make Docker workflows more adjustable #42

Closed
5 tasks done
veaceslavdoina opened this issue Jul 31, 2023 · 15 comments
Closed
5 tasks done

Make Docker workflows more adjustable #42

veaceslavdoina opened this issue Jul 31, 2023 · 15 comments
Assignees

Comments

@veaceslavdoina
Copy link
Contributor

veaceslavdoina commented Jul 31, 2023

Intro

The idea is coming from @emizzle and basically it is about to make existing Docker workflow more adjustable by passing build parameters and also use latest tag for every build

  • Create a reusable workflow based on an existing one
  • Create latest tag for all builds and distinguish them by name
    nim-codex:latest            - latest release
    nim-codex:latest-dist-tests - latest build for dist-tests
    nim-codex:latest-custom     - latest build for custom
    
  • Always use same DockerHub repository and distinguish images by tags, like
    nim-codex:latest
    nim-codex:latest-dist-tests
    

And we also should take care about how to identify which GitHub commit was used for the latest tag. One of the way to do that is to push tags with short sha as well, like we are doing it right now.

But in case when we would like to identify latest tag over the time it might be required to perform a check based on the image usage and push time.

Progress

@benbierens
Copy link
Contributor

benbierens commented Jul 31, 2023

You can currently change the codex image being used by setting the CODEXDOCKERIMAGE variable. If you want to use 'latest', set it to codexstorage/nim-codex:latest. (yes, it might make sense to make this the default at some point, and overrideable with the env-var! )
I will now check to make sure that when a latest image is used, the logs created will contain the info required to be able to identify the specific Codex version being used.

@veaceslavdoina veaceslavdoina self-assigned this Aug 1, 2023
@emizzle
Copy link
Contributor

emizzle commented Aug 8, 2023

Any progress on this? It would be really convenient if :lastest referenced latest master of the respective repository.

@veaceslavdoina
Copy link
Contributor Author

veaceslavdoina commented Aug 8, 2023

Any progress on this? It would be really convenient if :lastest referenced latest master of the respective repository.

Not yet, sorry for such a delay. Will change my priorities, but need to finish some stuff around.

@veaceslavdoina
Copy link
Contributor Author

There is another way to optimize workflows using Creating a composite action. But accordingly to the How to start using reusable workflows with GitHub Actions, they are limited for usage with Secrets, Platforms, Conditions and in our case we need to push images using secrets and create builds for amd64/arm64.
We are using composite action in nim-codex/actions/nimbus-build-system.

Information about composite action limitations maybe not up to date, but we are continuing with reusable workflows.

@veaceslavdoina
Copy link
Contributor Author

This is exactly our case - Multi-platform image with GitHub Actions/Distribute build across multiple runners, but for some reason it doesn't work as expected.

Need to check why push by digest works but image creation doesn't.

@veaceslavdoina
Copy link
Contributor Author

Looks there was an issue with DockerHub and I didn't show anything in the Tags section.

Test today in the morning was successful and we can see images pushed yesterday as well.

Screenshots Screenshot 2023-08-14 at 08 17 35 Screenshot 2023-08-14 at 08 17 24

So, we can adopt workflow to be reusable.

@emizzle
Copy link
Contributor

emizzle commented Aug 14, 2023

Thanks a lot, Slavas! I should be able to get around to testing this tomorrow 👍

@emizzle
Copy link
Contributor

emizzle commented Aug 15, 2023

I see, this was a test workflow to be applied to the images, got it. I'll test once the :latest images are up.

@veaceslavdoina
Copy link
Contributor Author

Eric, I'm working that now to make it reusable and to handle cases like

IMAGE_LATEST: true
IMAGE_SHA: false
IMAGE_SUFFIX: custom
IMAGE_SUFFIX_ONLATEST: false

Should be done today.

@veaceslavdoina
Copy link
Contributor Author

First PR is ready and it is about nim-codex.

It probably make sense to set tag_sha=true when we set tag_latest=true and in that way we will have latest tag and specific sha-123456 build as well.

# Configuration Git Commit Git Tag
1 IMAGE_LATEST=false
IMAGE_SHA=true
sha-af3cc6d 0.0.1
2 IMAGE_LATEST=true
IMAGE_SHA=false
latest 0.0.1
latest
3 IMAGE_LATEST=true
IMAGE_SHA=true
latest, sha-af3cc6d latest
0.0.1
4 IMAGE_LATEST=false
IMAGE_SHA=true
IMAGE_SUFFIX=custom
sha-af3cc6d-custom 0.0.1-custom
5 IMAGE_LATEST=true
IMAGE_SHA=false
IMAGE_SUFFIX=custom
latest-custom latest-custom
0.0.1-custom
6 IMAGE_LATEST=true
IMAGE_SHA=true
IMAGE_SUFFIX=custom
latest-custom, sha-af3cc6d-custom latest-custom, 0.0.1-custom

@veaceslavdoina
Copy link
Contributor Author

veaceslavdoina commented Aug 16, 2023

And some notes about latest tag usage

Docker

We should use --pull=always parameter to get the newer latest image at run

docker run --rm --pull=always codexstorage/test:latest codex --version

Kubernetes

We should set imagePullPolicy: Always at Pods creation to get the latest image

---
apiVersion: v1
kind: Pod
metadata:
  name: dist-tests-runner
  namespace: cs-codex-dist-tests
  labels:
    name: cs-codex-dist-tests
spec:
  containers:
  - name: cs-codex-dist-tests
    image: codexstorage/cs-codex-dist-tests:lates
    imagePullPolicy: Always

Also, we can just skip that policy and Kubernetes will use Always policy

if you omit the imagePullPolicy field, and the tag for the container image is :latest, imagePullPolicy is automatically set to Always;

But in our case with Dist-Tests, we are planning to have lates-dist-test tag and not just latest. And for that, we should set explicitly imagePullPolicy=Always and it is already done

ImagePullPolicy = "Always",

Reference

  1. Why can't I pull the newest image by using the latest tag?
  2. Kubernetes - Image pull policy
  3. Kubernetes - Default image pull policy
  4. Why We Should Use ‘latest’ Tag on Container Images

@emizzle
Copy link
Contributor

emizzle commented Aug 16, 2023

Very nice work @veaceslavdoina !! I will review this more closely tomorrow.

veaceslavdoina added a commit to codex-storage/codex-contracts-eth that referenced this issue Aug 16, 2023
veaceslavdoina added a commit to codex-storage/dist-tests-geth that referenced this issue Aug 16, 2023
veaceslavdoina added a commit to codex-storage/dist-tests-geth that referenced this issue Aug 16, 2023
veaceslavdoina added a commit to codex-storage/dist-tests-prometheus that referenced this issue Aug 16, 2023
veaceslavdoina added a commit to codex-storage/dist-tests-prometheus that referenced this issue Aug 16, 2023
@veaceslavdoina
Copy link
Contributor Author

veaceslavdoina commented Aug 16, 2023

One more question - should we consider to use just a custom word as a tag, when latest is disabled?

Configuration # Git Commit Git Tag
IMAGE_LATEST=false
IMAGE_SHA=true
IMAGE_SUFFIX=custom
current sha-af3cc6d-custom 0.0.1-custom
idea sha-af3cc6d-custom, custom 0.0.1-custom, custom

I was mostly focused on the latest tag, but we probably can extend the current implementation in the future if it will be required.

The question is coming from the comparison of the latest-custom vs just custom.

@emizzle
Copy link
Contributor

emizzle commented Aug 17, 2023

It probably make sense to set tag_sha=true when we set tag_latest=true and in that way we will have latest tag and specific sha-123456 build as well.

I definitely agree with this, as it will provide the ability to downgrade in case latest is broken.

One more question - should we consider to use just a custom word as a tag, when latest is disabled?

As long as the sha + custom label exists, eg sha-af3cc6d-custom, then I don't think we also need an image with the custom word as the tag, as it would be confusing: a user may ask themselves, "Should I use the latest-custom or custom image for my application?", where they will not know if both images point to the same commits or not, and the reality is they wouldn't.

My opinion is that we would have a maximum of 4 images produced depending on the config params:

# Tag Description Configuration
1 latest image built with the latest commit on master IMAGE_LATEST=true
2 sha-af3cc6d image built with commit af3cc6d IMAGE_SHA=true
3 latest-custom image built with the latest commit on master and custom build/run parameters IMAGE_LATEST=true and IMAGE_SUFFIX=custom
4 sha-af3cc6d-custom image built with commit af3cc6d and custom build/run parameters IMAGE_SUFFIX=custom

veaceslavdoina added a commit to codex-storage/codex-contracts-eth that referenced this issue Aug 17, 2023
@veaceslavdoina
Copy link
Contributor Author

Looks like I've already did it and replaced IMAGE_* with TAG_*, so no refactoring is needed.

All PR's were approved and merged and we can consider that as solved for now.

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

No branches or pull requests

3 participants