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.

Sign up for GitHub

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

Route to kubevirt VMs using infra id as service label selector #2092

Merged
merged 1 commit into from
Mar 19, 2023
Merged

Route to kubevirt VMs using infra id as service label selector #2092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/v1alpha1/hostedcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@ type PlatformSpec struct {
}

// KubevirtPlatformSpec specifies configuration for kubevirt guest cluster installations
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.generateID) || has(self.generateID)", message="Kubevirt GenerateID is required once set"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it mean "is required once set" ?
if GenerateID is required, it must not be empty or missing in the first place.
so there is no meaning to "once set" in my opinion.

and same for the validation message below - "is immutable once set" - if that field can't be empty when the resource is created, the value is set on creation, and the "once set" is superfluousץ

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's an explanation, and it's where i took the example from. https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/#immutablility-after-first-modification

The idea is that I want GenerateID to be allowed to be set exactly once, and never changed after that. Including removing the value entirely.

The line you reference !has(oldSelf.generateID) || has(self.generateID) is kind of odd. I believe what it's saying is that it's okay for the value to be set only when the old value was empty.

type KubevirtPlatformSpec struct {
// BaseDomainPassthrough toggles whether or not an automatically
// generated base domain for the guest cluster should be used that
Expand All @@ -703,6 +704,14 @@ type KubevirtPlatformSpec struct {
// +optional
// +immutable
BaseDomainPassthrough *bool `json:"baseDomainPassthrough,omitempty"`

// GenerateID is used to uniquely apply a name suffix to resources associated with
// kubevirt infrastructure resources
// +kubebuilder:validation:Optional
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Kubevirt GenerateID is immutable once set"
// +kubebuilder:validation:MaxLength=11
// +optional
GenerateID string `json:"generateID,omitempty"`
}

// AgentPlatformSpec specifies configuration for agent-based installations.
Expand Down
17 changes: 17 additions & 0 deletions api/v1beta1/hostedcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@ const (
// SilenceClusterAlertsLabel is a label that can be used by consumers to indicate
// alerts from a cluster can be silenced or ignored
SilenceClusterAlertsLabel = "hypershift.openshift.io/silence-cluster-alerts"

// InfraIDLabel is a label that indicates the hosted cluster's infra id
// that the resource is associated with.
InfraIDLabel = "hypershift.openshift.io/infra-id"

// NodePoolNameLabel is a label that indicates the name of the node pool
// a resource is associated with
NodePoolNameLabel = "hypershift.openshift.io/nodepool-name"
)

// HostedClusterSpec is the desired behavior of a HostedCluster.
Expand Down Expand Up @@ -656,6 +664,7 @@ type PlatformSpec struct {
}

// KubevirtPlatformSpec specifies configuration for kubevirt guest cluster installations
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.generateID) || has(self.generateID)", message="Kubevirt GenerateID is required once set"
type KubevirtPlatformSpec struct {
// BaseDomainPassthrough toggles whether or not an automatically
// generated base domain for the guest cluster should be used that
Expand All @@ -681,6 +690,14 @@ type KubevirtPlatformSpec struct {
// +optional
// +immutable
BaseDomainPassthrough *bool `json:"baseDomainPassthrough,omitempty"`

// GenerateID is used to uniquely apply a name suffix to resources associated with
// kubevirt infrastructure resources
// +kubebuilder:validation:Optional
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Kubevirt GenerateID is immutable once set"
// +kubebuilder:validation:MaxLength=11
// +optional
GenerateID string `json:"generateID,omitempty"`
}

// AgentPlatformSpec specifies configuration for agent-based installations.
Expand Down
22 changes: 22 additions & 0 deletions cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedclusters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2675,7 +2675,18 @@ spec:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name suffix
to resources associated with kubevirt infrastructure resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down Expand Up @@ -6258,7 +6269,18 @@ spec:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name suffix
to resources associated with kubevirt infrastructure resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down
22 changes: 22 additions & 0 deletions cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedcontrolplanes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2673,7 +2673,18 @@ spec:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name suffix
to resources associated with kubevirt infrastructure resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down Expand Up @@ -6249,7 +6260,18 @@ spec:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name suffix
to resources associated with kubevirt infrastructure resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down
9 changes: 7 additions & 2 deletions control-plane-operator/controllers/hostedcontrolplane/cloud/kubevirt/providerconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package kubevirt

import (
hyperv1 "github.com/openshift/hypershift/api/v1beta1"
"gopkg.in/yaml.v2"
)

Expand All @@ -15,6 +16,7 @@ type CloudConfig struct {
LoadBalancer LoadBalancerConfig `yaml:"loadBalancer"`
InstancesV2 InstancesV2Config `yaml:"instancesV2"`
Namespace string `yaml:"namespace"`
InfraLabels map[string]string `yaml:"infraLabels"`
}

type LoadBalancerConfig struct {
Expand All @@ -39,7 +41,7 @@ func (c *CloudConfig) serialize() (string, error) {
return string(out), nil
}

func cloudConfig(namespace string) CloudConfig {
func cloudConfig(hcp *hyperv1.HostedControlPlane) CloudConfig {
return CloudConfig{
LoadBalancer: LoadBalancerConfig{
Enabled: true,
Expand All @@ -48,6 +50,9 @@ func cloudConfig(namespace string) CloudConfig {
Enabled: true,
ZoneAndRegionEnabled: false,
},
Namespace: namespace,
Namespace: hcp.Namespace,
InfraLabels: map[string]string{
hyperv1.InfraIDLabel: hcp.Spec.InfraID,
},
}
}
2 changes: 1 addition & 1 deletion control-plane-operator/controllers/hostedcontrolplane/cloud/kubevirt/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

func ReconcileCloudConfig(cm *corev1.ConfigMap, hcp *hyperv1.HostedControlPlane) error {
cfg := cloudConfig(hcp.Namespace)
cfg := cloudConfig(hcp)
serializedCfg, err := cfg.serialize()
if err != nil {
return fmt.Errorf("failed to serialize cloudconfig: %w", err)
Expand Down
6 changes: 3 additions & 3 deletions control-plane-operator/controllers/hostedcontrolplane/csi/kubevirt/kubevirt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ func getContentsOrDie(file string) []byte {
return b
}

func reconcileInfraConfigMap(cm *corev1.ConfigMap) error {
func reconcileInfraConfigMap(cm *corev1.ConfigMap, infraID string) error {
cm.Data = map[string]string{
"infraClusterNamespace": cm.Namespace,
"infraClusterLabels": "",
"infraClusterLabels": fmt.Sprintf("%s=%s", hyperv1.InfraIDLabel, infraID),
}
return nil
}
Expand Down Expand Up @@ -420,7 +420,7 @@ func ReconcileInfra(client crclient.Client, hcp *hyperv1.HostedControlPlane, ctx

infraConfigMap := manifests.KubevirtCSIDriverInfraConfigMap(infraNamespace)
_, err = createOrUpdate(ctx, client, infraConfigMap, func() error {
return reconcileInfraConfigMap(infraConfigMap)
return reconcileInfraConfigMap(infraConfigMap, hcp.Spec.InfraID)
})
if err != nil {
return err
Expand Down
12 changes: 11 additions & 1 deletion ...rol-plane-operator/hostedclusterconfigoperator/controllers/resources/ingress/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ func ReconcileDefaultIngressPassthroughService(service *corev1.Service, defaultN
return fmt.Errorf("unable to detect default ingress NodePort https port")
}

if service.Labels == nil {
service.Labels = map[string]string{}
}
service.Spec.Ports = []corev1.ServicePort{
{
Name: "https-443",
Expand All @@ -121,10 +124,13 @@ func ReconcileDefaultIngressPassthroughService(service *corev1.Service, defaultN
},
}
service.Spec.Selector = map[string]string{
"kubevirt.io": "virt-launcher",
"kubevirt.io": "virt-launcher",
hyperv1.InfraIDLabel: hcp.Spec.InfraID,
}
service.Spec.Type = corev1.ServiceTypeClusterIP

service.Labels[hyperv1.InfraIDLabel] = hcp.Spec.InfraID

ownerRef.ApplyTo(service)

return nil
Expand All @@ -133,6 +139,9 @@ func ReconcileDefaultIngressPassthroughService(service *corev1.Service, defaultN
func ReconcileDefaultIngressPassthroughRoute(route *routev1.Route, cpService *corev1.Service, hcp *hyperv1.HostedControlPlane) error {
ownerRef := config.OwnerRefFrom(hcp)

if route.Labels == nil {
route.Labels = map[string]string{}
}
route.Spec.WildcardPolicy = routev1.WildcardPolicySubdomain
route.Spec.Host = fmt.Sprintf("https.apps.%s.%s", hcp.Name, hcp.Spec.DNS.BaseDomain)
route.Spec.TLS = &routev1.TLSConfig{
Expand All @@ -142,6 +151,7 @@ func ReconcileDefaultIngressPassthroughRoute(route *routev1.Route, cpService *co
Kind: "Service",
Name: cpService.Name,
}
route.Labels[hyperv1.InfraIDLabel] = hcp.Spec.InfraID

ownerRef.ApplyTo(route)

Expand Down
6 changes: 4 additions & 2 deletions ...rol-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,21 @@ func IngressDefaultIngressNodePortService() *corev1.Service {
}
}

const IngressDefaultIngressPassthroughServiceName = "default-ingress-passthrough-service"

func IngressDefaultIngressPassthroughService(namespace string) *corev1.Service {
return &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "default-ingress-passthrough-service",
Namespace: namespace,
},
}
}

const IngressDefaultIngressPassthroughRouteName = "default-ingress-passthrough-route"

func IngressDefaultIngressPassthroughRoute(namespace string) *routev1.Route {
return &routev1.Route{
ObjectMeta: metav1.ObjectMeta{
Name: "default-ingress-passthrough-route",
Namespace: namespace,
},
}
Expand Down
8 changes: 8 additions & 0 deletions control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,9 +777,17 @@ func (r *reconciler) reconcileIngressController(ctx context.Context, hcp *hyperv
// Manifests for infra/mgmt cluster passthrough service
cpService := manifests.IngressDefaultIngressPassthroughService(hcpNamespace)

cpService.Name = fmt.Sprintf("%s-%s",
manifests.IngressDefaultIngressPassthroughServiceName,
hcp.Spec.Platform.Kubevirt.GenerateID)

// Manifests for infra/mgmt cluster passthrough routes
cpPassthroughRoute := manifests.IngressDefaultIngressPassthroughRoute(hcpNamespace)

cpPassthroughRoute.Name = fmt.Sprintf("%s-%s",
manifests.IngressDefaultIngressPassthroughRouteName,
hcp.Spec.Platform.Kubevirt.GenerateID)

Comment on lines +780 to +790
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orenc1 i fixed the issue with the route and service not being unique by adding some unique generated strings to the end of these resources.

if _, err := r.CreateOrUpdate(ctx, r.cpClient, cpService, func() error {
return ingress.ReconcileDefaultIngressPassthroughService(cpService, defaultIngressNodePortService, hcp)
}); err != nil {
Expand Down
13 changes: 13 additions & 0 deletions docs/content/reference/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4946,6 +4946,19 @@ Apps: *.apps.guest.apps.mgmt-cluster.example.com</p>
<p>This is possible using OCP wildcard routes</p>
</td>
</tr>
<tr>
<td>
<code>generateID</code></br>
<em>
string
</em>
</td>
<td>
<em>(Optional)</em>
<p>GenerateID is used to uniquely apply a name suffix to resources associated with
kubevirt infrastructure resources</p>
</td>
</tr>
</tbody>
</table>
###KubevirtRootVolume { #hypershift.openshift.io/v1beta1.KubevirtRootVolume }
Expand Down
48 changes: 48 additions & 0 deletions hack/app-sre/saas_template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29925,7 +29925,19 @@ objects:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name
suffix to resources associated with kubevirt infrastructure
resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down Expand Up @@ -33577,7 +33589,19 @@ objects:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name
suffix to resources associated with kubevirt infrastructure
resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down Expand Up @@ -37356,7 +37380,19 @@ objects:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name
suffix to resources associated with kubevirt infrastructure
resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down Expand Up @@ -41002,7 +41038,19 @@ objects:
Cluster: guest.apps.mgmt-cluster.example.com Apps: *.apps.guest.apps.mgmt-cluster.example.com
\n This is possible using OCP wildcard routes"
type: boolean
generateID:
description: GenerateID is used to uniquely apply a name
suffix to resources associated with kubevirt infrastructure
resources
maxLength: 11
type: string
x-kubernetes-validations:
- message: Kubevirt GenerateID is immutable once set
rule: self == oldSelf
type: object
x-kubernetes-validations:
- message: Kubevirt GenerateID is required once set
rule: '!has(oldSelf.generateID) || has(self.generateID)'
powervs:
description: PowerVS specifies configuration for clusters running
on IBMCloud Power VS Service. This field is immutable. Once
Expand Down
Loading