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

(feat) Build and push container images #2376

Closed
wants to merge 2 commits into from

Conversation

aledegano
Copy link
Contributor

Create CD job to build and publish the container images for acceptance-tests, init-realm and certificates when they change any time a commit is merged in the master branch.

This CD job allows to deploy the Renku chart directly from this Git repository (e.g. skipping the chart repository) for those instances that want to deploy the tip of the master branch of Renku (e.g. dev.renku.ch).

Part of swissdatasciencecenter/terraform-renku#139.

@aledegano aledegano temporarily deployed to ci-renku-2376 December 3, 2021 09:50 Inactive
@aledegano aledegano temporarily deployed to ci-renku-2376 December 3, 2021 09:51 Inactive
@aledegano aledegano force-pushed the build_docker_images_on_merges branch from 6b8bc74 to 6f7756c Compare December 3, 2021 10:12
@aledegano aledegano temporarily deployed to ci-renku-2376 December 3, 2021 10:12 Inactive
@aledegano aledegano force-pushed the build_docker_images_on_merges branch from 6f7756c to dbcbec8 Compare December 3, 2021 10:29
@aledegano aledegano temporarily deployed to ci-renku-2376 December 3, 2021 10:29 Inactive
@aledegano aledegano force-pushed the build_docker_images_on_merges branch from dbcbec8 to 062cd81 Compare December 3, 2021 10:30
@aledegano aledegano temporarily deployed to ci-renku-2376 December 3, 2021 10:31 Inactive
Create CD job to build and publish the container images
for acceptance-tests, init-realm and certificates when
they change any time a commit is merged in the master
branch.

This CD job allows to deploy the Renku chart directly
from this Git repository (e.g. skipping the chart repository)
for those instances that want to deploy the tip of the
master branch of Renku (e.g. dev.renku.ch).

Part of SwissDataScienceCenter/terraform-renku#139.
@aledegano aledegano force-pushed the build_docker_images_on_merges branch from 062cd81 to 2ac14e2 Compare December 3, 2021 10:41
@aledegano aledegano temporarily deployed to ci-renku-2376 December 3, 2021 10:41 Inactive
@aledegano aledegano marked this pull request as ready for review December 3, 2021 10:58
@aledegano aledegano requested a review from a team as a code owner December 3, 2021 10:58
@rokroskar
Copy link
Member

so this basically avoids a chartpress for deploying master?

@rokroskar
Copy link
Member

Note that chartpress will actually check which images are already in the registry, so this will just speed up all builds and we don't really need to change the deploy actions.

@aledegano
Copy link
Contributor Author

Yes, this is to avoid chartpress. Specifically the reason I want to bypass chartpress is that it does not allow to specify latest as tag (or any other non-semver tag), thus once the images are published under that mandatory (semver) tag I would need to update the Helm chart by either pushing it back here in the repo (with the values file updated) or publishing the chart.
IMO both solution creates a lot of noise and cruft for little value.
Note that the actions I have implemented here also do not re-build/re-push the images that have not changed (I have tested this earlier). Finally once Flux will be responsible of deploying on dev.renku.ch we will be able to remove entirely the workflow "Deploy and Test", so the final amount of workflow should stay the same.

@rokroskar
Copy link
Member

Specifically the reason I want to bypass chartpress is that it does not allow to specify latest as tag (or any other non-semver tag), thus once the images are published under that mandatory (semver) tag I would need to update the Helm chart by either pushing it back here in the repo (with the values file updated) or publishing the chart.

Using latest is begging for (difficult to debug) trouble, the most obvious problem being "concurrency" issues (e.g. overlapping deploys pushing the same tag). In any case, you can set any tag you want with chartpress. You can always leave the chart as is and override the image tag values by using --set with helm.

Finally once Flux will be responsible of deploying on dev.renku.ch we will be able to remove entirely the workflow "Deploy and Test", so the final amount of workflow should stay the same.

Definitely not the test part - we want still to be testing the deployed versions.

@ableuler
Copy link
Contributor

ableuler commented Dec 6, 2021

Using latest is begging for (difficult to debug) trouble, the most obvious problem being "concurrency" issues (e.g. overlapping deploys pushing the same tag

I don't see this as a huge problem since these changes only affect our CD deployment at dev.renku.ch but not the CI deployments, but I guess there could be scenarios where this might happen. @rokroskar would you rather just publish a dev version of the renku chart after every merge to master and deploy that one?

@rokroskar
Copy link
Member

That's what we used to do in the past. There is really no obvious downside to that imho except that the list of published charts at https://swissdatasciencecenter.github.io/helm-charts/ gets very long. The problem with "latest" is that you just don't really know what it is - what if a number of deploys fail before the image publish step and you are trying to debug the CD deployment - you'll need to find the last commit for which the pipeline passed to have an idea of what is actually running, and you might not even realize it. Since we use dev.renku.ch for testing the next user-facing version of the platform, we need some more robustness there. What's wrong with using the deploy script that the action is using? That is made exactly for this reason (to easily deploy from any mixture of versions). The alternative is just to override the image tags - that also doesn't seem too difficult.

@aledegano
Copy link
Contributor Author

That's what we used to do in the past. There is really no obvious downside to that imho except that the list of published charts at https://swissdatasciencecenter.github.io/helm-charts/ gets very long. The problem with "latest" is that you just don't really know what it is - what if a number of deploys fail before the image publish step and you are trying to debug the CD deployment - you'll need to find the last commit for which the pipeline passed to have an idea of what is actually running, and you might not even realize it. Since we use dev.renku.ch for testing the next user-facing version of the platform, we need some more robustness there. What's wrong with using the deploy script that the action is using? That is made exactly for this reason (to easily deploy from any mixture of versions). The alternative is just to override the image tags - that also doesn't seem too difficult.

My only concern was exactly that the list of releases gets very long, but if you don't think that's a problem I will go with that. Overriding the image tags is easy, it's hard to use that tag in the workflow that we use to have Flux deploy the charts. At that point is a lot easier to publish a chart for every merge and deploy all of those charts in dev.renku.ch as they become available.
I'm closing this PR.

@aledegano aledegano closed this Dec 6, 2021
@rokroskar
Copy link
Member

I've pruned the charts manually maybe a handful of times in the past few years so I wouldn't say it's a huge deal - one could certainly design some automation around this. But we have relatively few merges to master now in comparison to before when every merge to master in all the other repos also updated master on renku.

@aledegano aledegano deleted the build_docker_images_on_merges branch December 6, 2021 10:06
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.

None yet

3 participants