Polling ubi images - Githubissues

pacostas commented 1 year ago

Related issues

One of the steps (add USN polling mechanism back in) on: https://github.com/paketo-community/ubi-base-stack/issues/2

Summary

This PR polls on ubi images, and in case there is a new one, it creates a release and push the new images to the registry. More specifically, it checks the hash codes of build, run , run nodejs 16 and run nodesjs 18 ubi images, and if they differ from the current ones, it triggers the ./scripts/create.sh to regenerate the .oci files, creates a new release and pushes the new images to the registry when the push-image job gets triggered. Similar to what the create-release.yml github workflow https://github.com/paketo-buildpacks/jammy-base-stack/blob/main/.github/workflows/create-release.yml does.

Checklist

[x] I have viewed, signed, and submitted the Contributor License Agreement.
[x] I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
[x] I have added an integration test, if necessary.
[x] I have reviewed the styleguide for guidance on my code quality.
[x] I'm happy with the commit history on this PR (I have rebased/squashed as needed).

Pending tasks

[x] How to push images to the registry (push-image.yml file)
[x] Description is being view as html code
[x] Updating in case there is new ubi images (so far we check only the minimal, the 16-minimal ad the 18-minimal)

pacostas commented 1 year ago

Pending on how and with which tags to push images to the registry. ATM the push images job fails

robdimsdale commented 1 year ago

I know this is only draft, but I wanted to give some high-level feedback.

I think this will work, but I'm not familiar enough with UBI images to know if you can rely on the hash for each image to deterministically predict the output image. For example, I know that we can't do this for the Ubuntu stacks because there's no easy way to predict all the transitive dependencies that are pulled in during the apt install process.

Additionally, because the changes you're making are to the create-release workflow file, you'd also have to add them to the .syncignore file, otherwise they'll be overwritten next time the sync workflow runs. Also, this means that you won't get any of the upstream updates to the workflow files. That may be fine for you but I just wanted to explicitly call it out.

If you did want to find a way to keep in sync with the upstream file, we'd need to abstract the create-release workflow to support multiple stack types. That isn't necessarily as hard as it sounds, as we can move a bunch of the logic from the workflow into scripts in each repo - essentially defining an interface that each repository conforms to - and then the Ubuntu stacks can have one set of scripts and the UBI stacks another.

pacostas commented 1 year ago

Thank you for your feedback.

The Red Hat approach is to use the latest ubi images, so in terms of predictability, this is something that is being taken into account during the image construction/publishing.

Thank you for the suggestion, added the .syncignore file. Also I see that almost all the repos use it, so we can have the sync with the upstream file as a future improvement.

pacostas commented 1 year ago

@robdimsdale Do you use any formatter tool/configuration for the .yaml files?

robdimsdale commented 1 year ago

Do you use any formatter tool/configuration for the .yaml files?

We have yamllint set up in the shared github config. It lints all the yaml files in that repository. So we lint them at source - when they are added to the shared config - rather than downstream in each individual repo (like this one).

mhdawson commented 1 year ago

I think this will work, but I'm not familiar enough with UBI images to know if you can rely on the hash for each image to deterministically predict the output image. For example, I know that we can't do this for the Ubuntu stacks because there's no easy way to predict all the transitive dependencies that are pulled in during the apt install process.

This is worth a bit of discussion so that @robdimsdale you know what we are thinking and that you agree it makes sense in our case.

We understand that for a given buildpack the model we have for ubi will not reproduce exactly the same result even if it used on the same inputs. That is both known and in line with the distro model.

One of the contributors to this that the RHEL node.js is distrubted as rpms and the entension uses dnf (the ubi equivalent to apt) to install node.js and related packages. The default model for a dnf install is that it gives you the latest version versus a specific one. In the ubi/distro model that is the intention as you get security fixes and it keeps you up to date. The result is that during the extension phase the build image ends up getting the latest version of Node.js (for a given major line requested). We've set up extension so that if the end user requests a version of Node.js that cannot be satisfied by that they will get an error.

In the extension, after build, the resulting artifacts are layered on top of the run image, which the extension switches to match the what was used in the build phase. In that case we want the latest version of the run image for the LTS version selected (for example paketo-community/run-nodejs-16). The switching does not include any additional dnf installs, it just uses wat is present in the paketo-community/run-nodejs-16, which in turn should just be the ubi Node.js container with some additional labels/metadata.

Every time the Node.js and related rpms are updated, we publish new Node.js containers which include those updated rpms. At the same time any time there are security fixes in the underlying ubi base container we also build/publish new ubi Node.js containers. Therefore to get the latest Node.js versions within an LTS line and to pick up security fixes we want the images related to the ubi-base-stack to be build/published to pick up those changes.

This motivates our thinking on how to drive updates of the stack.

If there are no new containers then we know they are up to date and no security patches are availble for them.
If there are new containers then either there were security updates in the underlying ubi image they are based on, or there is an update to the Node.js components

@robdimsdale, does this make sense or is there a subtlety or concern that we've overlooked? Would like to make sure we have your thoughts since you have the existing experience related to how updates are done for stacks.

mhdawson commented 1 year ago

Additionally, because the changes you're making are to the create-release workflow file, you'd also have to add them to the .syncignore file, otherwise they'll be overwritten next time the sync workflow runs. Also, this means that you won't get any of the upstream updates to the workflow files. That may be fine for you but I just wanted to explicitly call it out.

That is something I missed in earlier PRs. The create.sh scripts is already customised. I think we've just not enabled the updating common files but we do want to do that. Might do that in a separate PR where we add in the excludes and enable the automation.

mhdawson commented 1 year ago

I created this PR to enable additional automation and make sure we don't update custom files like create-release - https://github.com/paketo-community/ubi-base-stack/pull/9/

The plan is that once we have the stack/extensions/builders fully up and running then we'll make a pass through to see if/how we can integrate the changes in the files we've customized back into common versions.

pacostas commented 1 year ago

Additionally, because the changes you're making are to the create-release workflow file, you'd also have to add them to the .syncignore file, otherwise they'll be overwritten next time the sync workflow runs. Also, this means that you won't get any of the upstream updates to the workflow files. That may be fine for you but I just wanted to explicitly call it out.

That is something I missed in earlier PRs. The create.sh scripts is already customised. I think we've just not enabled the updating common files but we do want to do that. Might do that in a separate PR where we add in the excludes and enable the automation.

What I can do then, is to not commit on this pr the .syncignore file and see on the next PR form the workflow sync what will be generated hence what we might have to patch/align

pacostas commented 1 year ago

Pending on how and with which tags to push images to the registry. ATM the push images job fails

On this one, I dont have something else to add, as I dont know how and where do we push the images

robdimsdale commented 1 year ago

@mhdawson thanks for the detailed explanation. I think I mostly follow. If I understand you correctly, you're saying that the image corresponding to installing UBI nodejs container is sufficiently deterministic to allow you to have confidence that you're not missing updates.

Specifically:

If the hash hasn't changed then you can be sure there aren't any updates (i.e. there are no false-negatives)
However, you can't be sure that if the hash has changed that there are updates (i.e. you have the potential for false-positives)
But you can defend against that by only rebuilding if the hash has changed.

I don't think I have any concerns - assuming my understanding above is accurate. I don't have enough experience with them to know whether that's the case, so I trust you on that 😄

mhdawson commented 1 year ago

@robdimsdale thanks for taking a look at my explanation. I think if the hash has changed we want to pull in whatever was updated, I don't think there can be false positives. Either the base ubi image was updated or the Node.js related components installed in the extension have been updated. A new image for the ubi Node.js images implies one of those two.

pacostas commented 1 year ago

Removed the create-stack action https://github.com/paketo-community/ubi-base-stack/pull/7/commits/c0a675c1340c5738b3d9fe28c368a976b18fb1d8 , as it has been removed from the github-config https://github.com/paketo-buildpacks/github-config/pull/802

mhdawson commented 1 year ago

Did another pass through create-release.yml and made a few small comments, otherwise looks good.

I don't think we should need any changes in push-image.yml. I think those were most likely done as part of debugging and were not intended to be in the PR.

pacostas commented 12 months ago

Did another pass through create-release.yml and made a few small comments, otherwise looks good.

I don't think we should need any changes in push-image.yml. I think those were most likely done as part of debugging and were not intended to be in the PR.

Reverted the changes on this commit https://github.com/paketo-community/ubi-base-stack/pull/7/commits/86be926140cacbb6e515187eebc10d7dfdc88797

sophiewigmore commented 11 months ago

The overall mechanism here makes sense to me, I have a few small comments though

robdimsdale commented 11 months ago

We'll also need to update the create-stack references in both the Test PR and Create Release workflows, as the action has been replaced with inline invocation of ./scripts/create-stack.sh.

pacostas commented 11 months ago

We'll also need to update the create-stack references in both the Test PR and Create Release workflows, as the action has been replaced with inline invocation of ./scripts/create-stack.sh.

Invoking the create stack script without using the deprecated github action is done on this pr https://github.com/paketo-community/ubi-base-stack/pull/7/files#diff-8ac33fe295df086d3e55df1eb01194819d34cc0f5f54076dba96ceac0e40bd79R145-R146

robdimsdale commented 11 months ago

We'll also need to update the create-stack references in both the Test PR and Create Release workflows, as the action has been replaced with inline invocation of ./scripts/create-stack.sh.

Invoking the create stack script without using the deprecated github action is done on this pr https://github.com/paketo-community/ubi-base-stack/pull/7/files#diff-8ac33fe295df086d3e55df1eb01194819d34cc0f5f54076dba96ceac0e40bd79R145-R146

Right, for the create-release.yml workflow. We also need to update it in the test-pull-request.yml workflow, as the PR tests are failing with the following error:

Error: Can't find 'action.yml', 'action.yaml' or 'Dockerfile' for action 'paketo-buildpacks/github-config/actions/stack/create-stack@main'.

pacostas commented 11 months ago

We'll also need to update the create-stack references in both the Test PR and Create Release workflows, as the action has been replaced with inline invocation of ./scripts/create-stack.sh.

Invoking the create stack script without using the deprecated github action is done on this pr https://github.com/paketo-community/ubi-base-stack/pull/7/files#diff-8ac33fe295df086d3e55df1eb01194819d34cc0f5f54076dba96ceac0e40bd79R145-R146

Right, for the create-release.yml workflow. We also need to update it in the test-pull-request.yml workflow, as the PR tests are failing with the following error:
Error: Can't find 'action.yml', 'action.yaml' or 'Dockerfile' for action 'paketo-buildpacks/github-config/actions/stack/create-stack@main'.

I had to rebase and force push to fix this on test-pull-request.yml as I didnt have these changes on this branch here is the commit https://github.com/paketo-community/ubi-base-stack/pull/7/commits/d4b77d6b204db77bf856131c7400bbf526f53426

paketo-community / ubi-base-stack

Polling ubi images #7

Related issues

Summary

Checklist

Pending tasks