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

Use matrix strategy for CI/CD #402

Merged
merged 1 commit into from
Sep 12, 2024
Merged

Conversation

perfectra1n
Copy link
Contributor

@perfectra1n perfectra1n commented Sep 8, 2024

  • Use matrix strategy for CI/CD
  • Fix the start-docker.sh file to use sh instead of bash because Alpine doesn’t have bash.
  • Docker health checks in CI/CD for both Dockerfiles
  • Remove Alpine-specific Action YAML

Run:
https://github.com/perfectra1n/Notes/actions/runs/10757405589

GHCR:
image
image

DockerHub:
image

@perfectra1n perfectra1n changed the title Use matrix strategy for CI/CD [WIP] Use matrix strategy for CI/CD Sep 8, 2024
fix the digests missing

tweak dockerhub digests 1

Revert "tweak dockerhub digests 1"

This reverts commit 3542125d6035d2330fe1075682b046133568137d.

Revert "fix the digests missing"

This reverts commit 01954e5687549586233d73339b74e03e2182a339.

give this a shot then

add short sha

fix sha tag names
@perfectra1n perfectra1n changed the title [WIP] Use matrix strategy for CI/CD Use matrix strategy for CI/CD Sep 8, 2024
@perfectra1n
Copy link
Contributor Author

perfectra1n commented Sep 8, 2024

Getting attestations to work is also going to be...extremely difficult with this setup. If you think they're highly needed, I can keep working away at it, perhaps in another PR. I'm definitely going to need a couple days away from this now that it's working LOL

@JYC333
Copy link

JYC333 commented Sep 9, 2024

Great work! Have some questions:

  1. There is an unknow arch in ghcr, do you know where it comes from?
  2. Does test build need to test in different platforms? Not sure is that needed.
  3. I see in DockerHub triliumnext/notes has many tags with hash tag, do we need those tags? My feeling is we only need version tag and nightly build tag in DockerHub.

@perfectra1n
Copy link
Contributor Author

perfectra1n commented Sep 9, 2024

  1. Well I've gotta say I don't think I've ever seen this before:
docker pull ghcr.io/perfectra1n/notes:sha256-416ebcdb1430f74701a0af8eed77b0027254d976@sha256:82baed2479e317919dd76e05374b8988bb82012fe490d8b8d9287adfea97b1da
ghcr.io/perfectra1n/notes@sha256:82baed2479e317919dd76e05374b8988bb82012fe490d8b8d9287adfea97b1da: Pulling from perfectra1n/notes
unsupported media type application/vnd.in-toto+json

It's in our current builds. I'm assuming it has something to do with buildx creating an attestation, but no systems will pull that image as it's not a valid architecture. I wasn't able to stop it from being created with setting BUILDX_NO_DEFAULT_ATTESTATIONS. I don't think it's causing any harm, and is being used for DockerHub, I believe.

  1. This PR doesn't test the different architectures in the pipeline, it tests the two different Dockerfiles. I think that it's super important that we test the containers that we're building, and all Dockerfiles that we're building from. We've already run into issues with the discrepancies between the two different Dockerfiles that we're building ((Bug report) No docker images available for AMD64 machines since Aug 30, 2024 #400). Unless you meant for us to test also the ARMv7 and ARM64 builds within the pipeline? I believe that just checking the health of each of the Dockerfile's containers is enough.
  2. That was there previously to me making these changes, where the associated SHA of the latest commit is added as a valid tag. Create nightly release action #327 from you is still open, so I'm not sure if it's been confirmed that we want to change the tags to what you described? I'm fine with doing so, if we want to.

@perfectra1n perfectra1n force-pushed the develop branch 3 times, most recently from 1bfbc8b to 114e780 Compare September 10, 2024 00:24
@JYC333
Copy link

JYC333 commented Sep 10, 2024

3. That was there previously to me making these changes, where the associated SHA of the latest commit is added as a valid tag. Create nightly release action #327 from you is still open, so I'm not sure if it's been confirmed that we want to change the tags to what you described? I'm fine with doing so, if we want to.

From my experience, having SHA as tag for image is quite useless, it's too troublesome for users to pull the image to based on the SHA tag and I haven't done this before. The other reason could be, for example, the image below shows the digest for v0.90.6-beta and sha-4106cc6 are the same. I guess the digest for tag sha-4106cc6 barely gonna be used. And it'll be much cleaner if we only have version tags on DockerHub, will also let people who checks triliumnext DockerHub feel that we are not too casual.
image

For the nightly release, we can have that after we merge #327. We can overwrite the digest under nightly tag.

And I also find that you are still pushing images to triliumnext/notes, not sure whether you are aware of that. We already have 10 pages of images/tags, but we only have 6 releases now, it's not very reasonable for me. And I think we don't need develop tag, nightly could be a good one after #327 is merged. It can update daily instead of pushing based on commits.
image

@perfectra1n
Copy link
Contributor Author

@eliandoran are you okay with removing the sha* tags that currently exist? I think that it would be nice to have the develop tag with the latest commit, while the nightly tag would be reserved for the container built at the end of the day, but I don't care too much either way...

And I also find that you are still pushing images to triliumnext/notes, not sure whether you are aware of that.

That's because the CI/CD that pushes to DockerHub is using my API key to push to the DockerHub organization, it's not me per-se, it's just my API key. Any development that I'm doing is being pushed to my fork.

What you've brought up is outside of the scope for this PR, however. This PR is just to use the matrix strategy. I think it would be better if another PR was opened (or previous merged/closed) that include the suggestions you bring up above.

@JYC333
Copy link

JYC333 commented Sep 10, 2024

I agree I'm a bit overthinking and increasing the scope of this PR. Create an issue to follow up this #412.

Otherwise, the changes look good to me.

@eliandoran
Copy link
Contributor

eliandoran commented Sep 10, 2024

@perfectra1n ,

@eliandoran are you okay with removing the sha* tags that currently exist? I think that it would be nice to have the develop tag with the latest commit, while the nightly tag would be reserved for the container built at the end of the day, but I don't care too much either way...

I think it would be nice to get rid of the SHA labels, yes. Of course , in a separate PR.

And I also find that you are still pushing images to triliumnext/notes, not sure whether you are aware of that.

That's because the CI/CD that pushes to DockerHub is using my API key to push to the DockerHub organization, it's not me per-se, it's just my API key. Any development that I'm doing is being pushed to my fork.

Is there a way to make it appear as if the org is pushing it instead of a particular person? I think that would make it seem more professional. I've done a similar change in the GitHub release process, since we are now releasing directly via GitHub Actions.

@perfectra1n
Copy link
Contributor Author

perfectra1n commented Sep 10, 2024

Yeah it looks like the "org" can't:
image
image

But we could create a user with a more professional name, add them to the DockerHub Organization, and push the images with their credentials if you would like :)

@perfectra1n
Copy link
Contributor Author

I've done a similar change in the GitHub release process, since we are now releasing directly via GitHub Actions.

Oh, hopefully I haven't created a conflict then with my changes 😬

@eliandoran eliandoran merged commit 3134ef7 into TriliumNext:develop Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants