gcp: Remove support for Licenses, only use public image #4696
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cgwalters
force-pushed
the
revert-gcp-licenses
branch
from
9582c9c to
908efe7
Compare
|
A simple way to look at this is GCP now always works how we do AWS, we just boot the AMI references, we don't clone them. |
cgwalters
force-pushed
the
revert-gcp-licenses
branch
from
908efe7 to
850c564
Compare
pkg/types/gcp/platform.go
Outdated
| // When set, this will cause the installer to copy the image into user's project. | ||
| // This option is incompatible with any mechanism that makes use of pre-built images | ||
| // such as the current env OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE | ||
| // Licenses support has been removed and must be empty. |
There was a problem hiding this comment.
This is a breaking change that we cannot make. We can deprecate the field, but we need to continue to support it.
There was a problem hiding this comment.
Compromise: we can detect the case where the only thing listed in the licenses is the nested virt, and silently ignore it because we do that by default.
There was a problem hiding this comment.
Updated now with that compromise! cc https://github.com/amorenoz from #3430
There was a problem hiding this comment.
If we had had that restriction to the nested virt license initially, this would be great. Unfortunately, we cannot add a restriction now.
There was a problem hiding this comment.
I totally understand the PoV that this API was exposed and hence we need to support it ~forever. If you feel strongly about it, feel free to close this PR.
But again I'm pretty sure no actual customers or CI systems are using anything except nested virt.
The history here is basically that originally for GCP, we cloned a disk image, just like how Azure works today. Then later, GCP cracked down on the allowable frequency of image creation and told us to use public bootable images, so we started uploading a public RHCOS base image. But we originally did that without the nested virt license, so the KubeVirt team added a PR to clone the image and add the nested virt license. And then finally, in 4.6 we fixed the CoreOS public image to add the nested virt license by default (which also, GCP recommended we do): coreos/coreos-assembler@73320cf So there's no need for any of this image cloning or license stuff anymore. For compatibility (with e.g. the KubeVirt CI) we silently ignore the `enable-vmx` licenses because it's implicitly enabled by default anyways. Copying the image to enable it is a pessimization in fact! Other licenses are a fatal error - I am not aware of any that anyone would want. And anyone who wants to do something truly custom can use the image override variable anyways; they don't *have* to use our Terraform code. Or they can hack the installer code. Prep for using the stream metadata in openshift#4582
cgwalters
force-pushed
the
revert-gcp-licenses
branch
from
850c564 to
647f110
Compare
openshift-ci-robot
added
the
needs-rebase
|
@cgwalters: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
/uncc |
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
@cgwalters: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
gcp: Ignore Licenses: vmx, only use public image
The history here is basically that originally for GCP, we
cloned a disk image, just like how Azure works today.
Then later, GCP cracked down on the allowable frequency of image
creation and told us to use public bootable images, so we started
uploading a public RHCOS base image.
But we originally did that without the nested virt license, so
the KubeVirt team added a PR to clone the image and add the
nested virt license.
And then finally, in 4.6 we fixed the CoreOS public image
to add the nested virt license by default (which also, GCP
recommended we do):
coreos/coreos-assembler@73320cf
So there's no need for any of this image cloning or license
stuff anymore. For compatibility (with e.g. the KubeVirt CI)
we silently ignore the
enable-vmxlicenses because it'simplicitly enabled by default anyways. Copying the image
to enable it is a pessimization in fact!
Other licenses are a fatal error - I am not aware of
any that anyone would want. And anyone who wants to
do something truly custom can use
the image override variable anyways; they don't have
to use our Terraform code. Or they can hack the installer
code.
Prep for using the stream metadata in
#4582