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

Add digest reference to KubeVirt image description #46

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Add digest reference to KubeVirt image description #46

merged 1 commit into from
Apr 15, 2022

Conversation

bgilbert
Copy link
Contributor

In the current design, the KubeVirt image is presumed to be a fully-qualified reference, including a container digest. But that implies that the user should reference the image by digest, when we'd prefer that they reference it by stream-specific tag.

Define the existing image field to contain the preferred form of fully-qualified reference for the image, which might involve either a
tag or a digest. We should still record the unique identifier of the current image (as we do for GCP images), so add a digest-ref field which always contains a fully-qualified reference with digest.

cc @rmohr @dustymabe

In the current design, the KubeVirt image is presumed to be a fully-
qualified reference, including a container digest.  But that implies that
the user should reference the image by digest, when we'd prefer that they
reference it by stream-specific tag.

Define the existing "image" field to contain the preferred form of
fully-qualified reference for the image, which might involve either a
tag or a digest.  We should still record the unique identifier of the
current image (as we do for GCP images), so add a "digest-ref" field
which always contains a fully-qualified reference with digest.
@cgwalters cgwalters merged commit f3a1ef6 into coreos:main Apr 15, 2022
@rmohr
Copy link
Member

rmohr commented Apr 15, 2022

In the current design, the KubeVirt image is presumed to be a fully-qualified reference, including a container digest. But that implies that the user should reference the image by digest, when we'd prefer that they reference it by stream-specific tag.

Define the existing image field to contain the preferred form of fully-qualified reference for the image, which might involve either a tag or a digest. We should still record the unique identifier of the current image (as we do for GCP images), so add a digest-ref field which always contains a fully-qualified reference with digest.

cc @rmohr @dustymabe

@bgilbert @cgwalters

I did not want to go too much into detail on the comment in coreos/fedora-coreos-pipeline#515 (comment), but I don't think this goes in the direction I intended. I intentianally only planned to provide a full qualified image reference with @sha256 digest and not tags. The image has a tag to avoid garbage collection on quay, but it has no special meaning. I was also ok with going with a moving tag like stable, next and so on to give people a way to easily find the latest, but I only see them as an easy way to clone a reproducible reference for production payloads or getting the new digest from it without going through coreos-installer.

I think there is a difference between downloading a fresh image from a well know location and using a container image registry. In contrast to a download location, an image reference can directly used on real workloads. As such, using tags (especially moving ones) is definitely not what anyone should do and normally does. What may be missing is a way in coreos-installer to directly get the full image reference instead of having to download the oci image, but the new focus on tags does not really look right.

Just to better explain the rationale behind not using latest or other moving tags directly:

  • It invalidates any testing i did with my payload, since the base can change between e.g. workload restarts
  • I may not have a clear way to go back to a working state because I don't know which image I exactly used

That does however not contradict the idea of updating fast, it just means that one has to do additional steps to pick up changes from fast-releasing upstream which brings potential CVE fixes:

  • cloning fresh imags automatically
  • running validation tests in CI
  • then updating the workloads to use the new base.

For the image stream metadata a tag does not provide any additional benefit to tha digest, because the stream info already references the artifact in a fully compatible and most exact way. A fresh fcos build would belong to a new stream file.

Sorry for the rather long response, but I wanted to at least have mentioned this in detail. Let me know what you think.

@bgilbert bgilbert deleted the kubevirt branch April 15, 2022 16:10
@bgilbert
Copy link
Contributor Author

@rmohr One of the main benefits of Fedora CoreOS is that it updates automatically. Users should be able to think of the OS as an appliance, and its version as an implementation detail: it exists, but they shouldn't need to pay attention to it. We do not support or recommend running old releases, since they don't contain the latest security updates and other important fixes.

As to testing: the recommended strategy for all users is to run a few testing and next nodes in clusters alongside stable nodes. Both testing and next represent the functionality that will eventually be promoted to stable, on different timescales: testing is the next stable, and next contains larger changes planned for further down the road. Any regressions should be reported to us so we can prevent those regressions from being promoted to stable. This approach also catches OS-level problems that CI alone may not catch, such as hardware-specific or load-specific behavior. And a replicated cluster should be able to tolerate occasional failures of the testing/next nodes.

Thus, our recommendation for KubeVirt users is the same as our recommendation on all other platforms: always launch the latest release. The implementation varies by platform though. For example:

  • On GCP, there's e.g. a fedora-coreos-stable "image family" that always points to the latest release. Users should launch new instances from that image family and they'll always get the latest.
  • AWS has no such facility, so we recommend that users always query the stream metadata when launching a new instance to find the ID of the latest image.
  • On bare metal, if coreos-installer is run from a package or container, it'll automatically download the latest install image. However, when installing from ISO or PXE media, coreos-installer uses an image embedded in the media, so it's up to the user to keep their install media up to date.

For the KubeVirt ContainerDisk, we're lucky to have a mechanism (image tags) that allows users to pull the latest image at launch time, without needing to manually query stream metadata for the latest ref. Therefore, we should emphasize it. The image digest will still be included in the stream metadata for those who nevertheless choose to pin a release.

To be clear, all of this applies specifically to FCOS. In RHCOS, the OpenShift installer pins the bootimage version that should be used. This PR defines the image field such that in RHCOS, we can point both fields directly to an image digest.

@dustymabe
Copy link
Member

@bgilbert
As to testing: the recommended strategy for all users is to run a few testing and next nodes in clusters alongside stable nodes.

Exactly. I think this is the key piece of the model that allows us to "follow latest" and still achieve all the sames goals that @rmohr mentioned in #46 (comment).

@rmohr in short, people can get still get all the qualification that they would be doing with the steps laid out in the last set of bullet points from #46 (comment) but they just do it on the testing stream. If their validation tests fail on the testing stream then they know they've got some time to find the root cause before it hits stable. Additionally the image digest information is in the stream metadata if they really prefer the other model or need to spin up old images.

@cgwalters
Copy link
Member

In RHCOS, the OpenShift installer pins the bootimage version that should be used. This PR defines the image field such that in RHCOS, we can point both fields directly to an image digest.

I think along with the strategy for openshift/enhancements#1032 we are going to be offering more flexibility to users of OCP/RHCOS, so I wouldn't necessarily assume that FCOS is always floating and RHCOS is always pinned. I think basically we want to offer both tags and digested pull specs for both.

Now, this thread heavily intersects with the GC policy issue - we haven't addressed that yet.

@rmohr
Copy link
Member

rmohr commented Apr 19, 2022

@bgilbert
As to testing: the recommended strategy for all users is to run a few testing and next nodes in clusters alongside stable nodes.

Exactly. I think this is the key piece of the model that allows us to "follow latest" and still achieve all the sames goals that @rmohr mentioned in #46 (comment).

@rmohr in short, people can get still get all the qualification that they would be doing with the steps laid out in the last set of bullet points from #46 (comment) but they just do it on the testing stream. If their validation tests fail on the testing stream then they know they've got some time to find the root cause before it hits stable. Additionally the image digest information is in the stream metadata if they really prefer the other model or need to spin up old images.

Since I am still not 100% sure that all implications are clear, here a few cases where people using a floating tag will run into unexpected downtime on their services in their k8s cluster. It is really crucial for me that we all talk about the same things, and that these side-effects are "wanted/expected":

  • I see my code failing on testing-stream CI on day X. On day X+1 testing gets promoted to stable. VMs get restarted or automatic scaling scales them. Suddenly my service does not come up due to e.g. a selinux change which I just detected last minute on the testing stream. The remaining VM instances are overloaded, my whole service is down.
  • I see my code failing on testing-stream CI on day X. I still have Y days left to resolve the issue. I can't figure out the issue in Y days. Suddenly my production workloads start to fail, although I reconfigured prod settings to not be affected by CVE Z fixed in latest fcos stream until I finally have resolved the issue.
  • I test in stable-stream CI based on the floating tag that everything is fine with CI on day X. Three hours later, after CI did all verifications it starts the rollout. On production I end up with a broken deployment, since the stable tag just moved to a different version during that time window.
  • After all CI passed, I do canary rollouts, allowing max-surge of 1. Weirdly the first new VM fails. My automation tries to do a rollback, but since I am always referncing the same moving tag, my automatic system does not even see a difference to where to roll back to.
  • I see a new issue in a fcos stream with my app. I fix it. The fix makes my app work with the new release X, but breaks compatibility with older versions. This is fine, since I expect to use the new version only when I roll out new versions of my software. Weirdly I end up in a situation where on some nodes the app works and on some it does not. It turns out, the issue is that on k8s nodes where the app already ran, an older image was already pulled 2 weeks ago and it is still used, while on newer nodes the pull happens right now.

Let me know if you see theses as potential issues.

@bgilbert
Copy link
Contributor Author

Thanks for the concrete examples.

  • I see my code failing on testing-stream CI on day X. On day X+1 testing gets promoted to stable.

If you're in this situation, one of a couple things has happened: either your code has been failing in testing CI for two weeks and you haven't reported it to us, or (infrequently) we've pushed an out-of-cycle update to testing to fix an important issue. In the latter case, we're paying extra attention to bug reports, so you should report the problem immediately to stop the promotion.

  • I see my code failing on testing-stream CI on day X. I still have Y days left to resolve the issue. I can't figure out the issue in Y days.

It's not solely your responsibility to diagnose OS regressions. Report the problem to us; we can and do revert/pin packages until problems can be fixed.

  • I test in stable-stream CI based on the floating tag that everything is fine with CI on day X. Three hours later, after CI did all verifications it starts the rollout. On production I end up with a broken deployment, since the stable tag just moved to a different version during that time window.

Again, this implies that you aren't testing on testing and aren't reporting problems to us.

  • After all CI passed, I do canary rollouts, allowing max-surge of 1. Weirdly the first new VM fails. My automation tries to do a rollback, but since I am always referncing the same moving tag, my automatic system does not even see a difference to where to roll back to.

If this is a canary rollout of your app, you can still roll back to the previous fixed tag. If it's a canary rollout of the host OS, and you can't do an on-node rollback (i.e., you're rolling out fresh nodes rather than upgrading in place), then yes, at that point you'll need to switch to a tag reference. But this also implies that the testing nodes failed to catch the issue.

  • I see a new issue in a fcos stream with my app. I fix it. The fix makes my app work with the new release X, but breaks compatibility with older versions. This is fine, since I expect to use the new version only when I roll out new versions of my software. Weirdly I end up in a situation where on some nodes the app works and on some it does not. It turns out, the issue is that on k8s nodes where the app already ran, an older image was already pulled 2 weeks ago and it is still used, while on newer nodes the pull happens right now.

If the new version of the OS has forward- and backward-incompatible changes, report those to us, since we're careful to avoid those. If the goal is just to take advantage of new functionality added in an OS release, then as long as you've waited 48-72 hours after a stable release before depending on any new behavior, all of your nodes should have automatically updated themselves to the new version.


I think part of the mismatch here is: you're thinking about the advanced case and we're thinking about the common case. If your cluster carefully controls the versions of your host OS, CIs everything together, and does frequent rollouts of host nodes, you may well decide to pin to version tags or image digests and have the cluster manage OS versions directly. The stream metadata will still provide enough information for you to do so. (In that case you should also turn off Zincati.) But in many cases, users are tempted to just pin to a random OS version and walk away, leaving their systems insecure in the long run. For those users, we provide an alternative model that still keeps their systems up to date.

@rmohr
Copy link
Member

rmohr commented Apr 20, 2022

I think part of the mismatch here is: you're thinking about the advanced case and we're thinking about the common case. If your cluster carefully controls the versions of your host OS, CIs everything together, and does frequent rollouts of host nodes, you may well decide to pin to version tags or image digests and have the cluster manage OS versions directly. The stream metadata will still provide enough information for you to do so. (In that case you should also turn off Zincati.) But in many cases, users are tempted to just pin to a random OS version and walk away, leaving their systems insecure in the long run. For those users, we provide an alternative model that still keeps their systems up to date.

Alright. Seems like all implications are well understood :) Thanks for the detailed response.

jlebon added a commit to jlebon/coreos-assembler that referenced this pull request May 8, 2023
We're not currently setting all the fields of the `kubevirt` image in
release metadata. Notably as per earlier discussions[[1]][[2]], we want
the `image` field to use a floating tag and the `digest-ref` field to
use a digest pullspec.

Determining the floating tag to use is a bit hacky right now, but safe:
we find in the list of pushed tags the one which doesn't have a build ID
reference to it. If we find multiple, we fail. In the future, we'll
probably want to strengthen this to be more explicit.

[1]: coreos/stream-metadata-go#46
[2]: coreos/fedora-coreos-tracker#1172

Closes: coreos#3456
jlebon added a commit to coreos/coreos-assembler that referenced this pull request May 8, 2023
We're not currently setting all the fields of the `kubevirt` image in
release metadata. Notably as per earlier discussions[[1]][[2]], we want
the `image` field to use a floating tag and the `digest-ref` field to
use a digest pullspec.

Determining the floating tag to use is a bit hacky right now, but safe:
we find in the list of pushed tags the one which doesn't have a build ID
reference to it. If we find multiple, we fail. In the future, we'll
probably want to strengthen this to be more explicit.

[1]: coreos/stream-metadata-go#46
[2]: coreos/fedora-coreos-tracker#1172

Closes: #3456
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.

4 participants