From 77234cc9830ab91cd0f52ff5587ee35b1b0cb6ff Mon Sep 17 00:00:00 2001 From: AnnaZivkovic Date: Thu, 3 Aug 2023 17:03:44 -0700 Subject: [PATCH] CORS-2660: GCP: deprecate the licenses field It has been at leas two years since the licence field has become obsolete due RHCOS GCP images already having nested virt licences by default. The licenses field is being kept for backwards compatibility --- data/data/gcp/cluster/main.tf | 23 +------------------ data/data/gcp/variables-gcp.tf | 17 -------------- .../install.openshift.io_installconfigs.yaml | 10 -------- pkg/asset/cluster/tfvars.go | 7 +----- pkg/asset/machines/gcp/machines.go | 4 +--- pkg/tfvars/gcp/gcp.go | 12 ---------- pkg/types/gcp/platform.go | 8 ------- pkg/types/gcp/validation/platform.go | 12 ---------- 8 files changed, 3 insertions(+), 90 deletions(-) diff --git a/data/data/gcp/cluster/main.tf b/data/data/gcp/cluster/main.tf index d862bd8a3d9..68e43886d93 100644 --- a/data/data/gcp/cluster/main.tf +++ b/data/data/gcp/cluster/main.tf @@ -3,7 +3,7 @@ locals { worker_subnet_cidr = cidrsubnet(var.machine_v4_cidrs[0], 1, 1) #worker subnet is a smaller subnet within the vnet. e.g., from /21 to /22 public_endpoints = var.gcp_publish_strategy == "External" ? true : false - gcp_image = var.gcp_preexisting_image ? var.gcp_image : google_compute_image.cluster[0].self_link + gcp_image = var.gcp_image description = "Created By OpenShift Installer" } @@ -81,24 +81,3 @@ module "dns" { gcp_extra_labels = var.gcp_extra_labels } -resource "google_compute_image" "cluster" { - count = var.gcp_preexisting_image ? 0 : 1 - description = local.description - - name = "${var.cluster_id}-rhcos-image" - - # See https://github.com/openshift/installer/issues/2546 - guest_os_features { - type = "SECURE_BOOT" - } - guest_os_features { - type = "UEFI_COMPATIBLE" - } - - raw_disk { - source = var.gcp_image_uri - } - - licenses = var.gcp_image_licenses - labels = var.gcp_extra_labels -} diff --git a/data/data/gcp/variables-gcp.tf b/data/data/gcp/variables-gcp.tf index fc86709b4f3..12fde4ccae6 100644 --- a/data/data/gcp/variables-gcp.tf +++ b/data/data/gcp/variables-gcp.tf @@ -53,11 +53,6 @@ variable "gcp_master_instance_type" { description = "Instance type for the master node(s). Example: `n1-standard-4`" } -variable "gcp_image_uri" { - type = string - description = "URL to Raw Image for all nodes. This is used in case a new image needs to be generated for the nodes." -} - variable "gcp_image" { type = string description = "URL to the Image for all nodes." @@ -69,12 +64,6 @@ variable "gcp_instance_service_account" { default = "" } -variable "gcp_preexisting_image" { - type = bool - default = true - description = "Specifies whether an existing GCP Image should be used or a new one created for installation" -} - variable "gcp_master_root_volume_type" { type = string description = "The type of volume for the root block device of master nodes." @@ -128,12 +117,6 @@ variable "gcp_publish_strategy" { description = "The cluster publishing strategy, either Internal or External" } -variable "gcp_image_licenses" { - type = list(string) - description = "The licenses to use when creating compute instances" - default = [] -} - variable "gcp_root_volume_kms_key_link" { type = string description = "The GCP self link of KMS key to encrypt the volume." diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index a3168fb43f8..baf2c2a0732 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -3178,16 +3178,6 @@ spec: type: string type: array type: object - licenses: - description: Licenses is a list of licenses to apply to the compute - images The value should a list of strings (https URLs only) - representing the license keys. 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 - items: - type: string - type: array network: description: Network specifies an existing VPC where the cluster should be created rather than provisioning a new one. diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 82f0a843d8a..fd8082ef028 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -490,18 +490,13 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { if img == nil { return fmt.Errorf("%s: No GCP build found", st.FormatPrefix(archName)) } - // For backwards compatibility, we generate this URL to the image (only applies to RHCOS, not FCOS/OKD) - // right now. It will only be used if nested virt or other licenses are enabled, which we - // really should deprecate and remove - xref https://github.com/openshift/installer/pull/4696 - imageURL := fmt.Sprintf("https://storage.googleapis.com/rhcos/rhcos/%s.tar.gz", img.Name) + data, err := gcptfvars.TFVars( gcptfvars.TFVarsSources{ Auth: auth, MasterConfigs: masterConfigs, WorkerConfigs: workerConfigs, CreateFirewallRules: createFirewallRules, - ImageURI: imageURL, - ImageLicenses: installConfig.Config.GCP.Licenses, PreexistingNetwork: preexistingnetwork, PublicZoneName: publicZoneName, PrivateZoneName: privateZoneName, diff --git a/pkg/asset/machines/gcp/machines.go b/pkg/asset/machines/gcp/machines.go index 746e96ddd02..cf24e9197a1 100644 --- a/pkg/asset/machines/gcp/machines.go +++ b/pkg/asset/machines/gcp/machines.go @@ -132,9 +132,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine func provider(clusterID string, platform *gcp.Platform, mpool *gcp.MachinePool, osImage string, azIdx int, role, userDataSecret string, credentialsMode types.CredentialsMode) (*machineapi.GCPMachineProviderSpec, error) { az := mpool.Zones[azIdx] - if len(platform.Licenses) > 0 { - osImage = fmt.Sprintf("%s-rhcos-image", clusterID) - } else if mpool.OSImage != nil { + if mpool.OSImage != nil { osImage = fmt.Sprintf("projects/%s/global/images/%s", mpool.OSImage.Project, mpool.OSImage.Name) } network, subnetwork, err := getNetworks(platform, clusterID, role) diff --git a/pkg/tfvars/gcp/gcp.go b/pkg/tfvars/gcp/gcp.go index f1feaf07444..1f367180139 100644 --- a/pkg/tfvars/gcp/gcp.go +++ b/pkg/tfvars/gcp/gcp.go @@ -33,11 +33,8 @@ type config struct { CreateFirewallRules bool `json:"gcp_create_firewall_rules"` MasterInstanceType string `json:"gcp_master_instance_type,omitempty"` MasterAvailabilityZones []string `json:"gcp_master_availability_zones"` - ImageURI string `json:"gcp_image_uri,omitempty"` Image string `json:"gcp_image,omitempty"` - PreexistingImage bool `json:"gcp_preexisting_image"` InstanceServiceAccount string `json:"gcp_instance_service_account,omitempty"` - ImageLicenses []string `json:"gcp_image_licenses,omitempty"` VolumeType string `json:"gcp_master_root_volume_type"` VolumeSize int64 `json:"gcp_master_root_volume_size"` VolumeKMSKeyLink string `json:"gcp_root_volume_kms_key_link"` @@ -59,8 +56,6 @@ type config struct { type TFVarsSources struct { Auth Auth CreateFirewallRules bool - ImageURI string - ImageLicenses []string MasterConfigs []*machineapi.GCPMachineProviderSpec WorkerConfigs []*machineapi.GCPMachineProviderSpec PublicZoneName string @@ -95,9 +90,7 @@ func TFVars(sources TFVarsSources) ([]byte, error) { MasterAvailabilityZones: masterAvailabilityZones, VolumeType: masterConfig.Disks[0].Type, VolumeSize: masterConfig.Disks[0].SizeGB, - ImageURI: sources.ImageURI, Image: masterConfig.Disks[0].Image, - ImageLicenses: sources.ImageLicenses, PublicZoneName: sources.PublicZoneName, PrivateZoneName: sources.PrivateZoneName, PublishStrategy: string(sources.PublishStrategy), @@ -112,11 +105,6 @@ func TFVars(sources TFVarsSources) ([]byte, error) { ExtraLabels: labels, } - cfg.PreexistingImage = true - if len(sources.ImageLicenses) > 0 { - cfg.PreexistingImage = false - } - if masterConfig.Disks[0].EncryptionKey != nil { cfg.VolumeKMSKeyLink = generateDiskEncryptionKeyLink(masterConfig.Disks[0].EncryptionKey, masterConfig.ProjectID) } diff --git a/pkg/types/gcp/platform.go b/pkg/types/gcp/platform.go index 319b0fda682..bf5658c0cbf 100644 --- a/pkg/types/gcp/platform.go +++ b/pkg/types/gcp/platform.go @@ -35,14 +35,6 @@ type Platform struct { // +optional ComputeSubnet string `json:"computeSubnet,omitempty"` - // Licenses is a list of licenses to apply to the compute images - // The value should a list of strings (https URLs only) representing the license keys. - // 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 - // +optional - Licenses []string `json:"licenses,omitempty"` - // userLabels has additional keys and values that the installer will add as // labels to all resources that it creates on GCP. Resources created by the // cluster itself may not include these labels. This is a TechPreview feature diff --git a/pkg/types/gcp/validation/platform.go b/pkg/types/gcp/validation/platform.go index 3a7a3a689fd..4d3e3ba9696 100644 --- a/pkg/types/gcp/validation/platform.go +++ b/pkg/types/gcp/validation/platform.go @@ -2,7 +2,6 @@ package validation import ( "fmt" - "os" "regexp" "sort" @@ -10,7 +9,6 @@ import ( "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/gcp" - "github.com/openshift/installer/pkg/validate" ) var ( @@ -127,16 +125,6 @@ func ValidatePlatform(p *gcp.Platform, fldPath *field.Path, ic *types.InstallCon allErrs = append(allErrs, field.Required(fldPath.Child("network"), "must provide a VPC network when supplying subnets")) } - if oi, ok := os.LookupEnv("OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE"); ok && oi != "" && len(p.Licenses) > 0 { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("licenses"), "the use of custom image licenses is forbidden if an OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE is specified")) - } - - for i, license := range p.Licenses { - if validate.URIWithProtocol(license, "https") != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("licenses").Index(i), license, "licenses must be URLs (https) only")) - } - } - // check if configured userLabels are valid. allErrs = append(allErrs, validateUserLabels(p.UserLabels, fldPath.Child("userLabels"))...)