Skip to content

Commit

Permalink
Merge pull request openshift#2620 from nunnatsa/cache-delete-edge-case
Browse files Browse the repository at this point in the history 
KubeVirt: Handle deletion of the cache DV on an edge case
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Jun 2, 2023
2 parents 65bc249 + 872ca34 commit 8cda39e
Show file tree
Hide file tree
Showing 13 changed files with 250 additions and 64 deletions.
9 changes: 7 additions & 2 deletions api/v1alpha1/nodepool_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,9 +856,14 @@ type KubeVirtNodePoolStatus struct {
// +optional
CacheName string `json:"cacheName,omitempty"`

// RemoteNamespace holds the namespace of the remote infra cluster, if defined
// Credentials shows the client credentials used when creating KubeVirt virtual machines.
// This filed is only exists when the KubeVirt virtual machines are being placed
// on a cluster separate from the one hosting the Hosted Control Plane components.
//
// The default behavior when Credentials is not defined is for the KubeVirt VMs to be placed on
// the same cluster and namespace as the Hosted Control Plane.
// +optional
RemoteNamespace string `json:"remoteNamespace,omitempty"`
Credentials *KubevirtPlatformCredentials `json:"credentials,omitempty"`
}

// Taint is as v1 Core but without TimeAdded.
Expand Down
7 changes: 6 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions api/v1beta1/nodepool_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -845,9 +845,14 @@ type KubeVirtNodePoolStatus struct {
// +optional
CacheName string `json:"cacheName,omitempty"`

// RemoteNamespace holds the namespace of the remote infra cluster, if defined
// Credentials shows the client credentials used when creating KubeVirt virtual machines.
// This filed is only exists when the KubeVirt virtual machines are being placed
// on a cluster separate from the one hosting the Hosted Control Plane components.
//
// The default behavior when Credentials is not defined is for the KubeVirt VMs to be placed on
// the same cluster and namespace as the Hosted Control Plane.
// +optional
RemoteNamespace string `json:"remoteNamespace,omitempty"`
Credentials *KubevirtPlatformCredentials `json:"credentials,omitempty"`
}

// Taint is as v1 Core but without TimeAdded.
Expand Down
7 changes: 6 additions & 1 deletion api/v1beta1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 68 additions & 8 deletions cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -876,10 +876,40 @@ spec:
description: CacheName holds the name of the cache DataVolume,
if exists
type: string
remoteNamespace:
description: RemoteNamespace holds the namespace of the remote
infra cluster, if defined
type: string
credentials:
description: "Credentials shows the client credentials used
when creating KubeVirt virtual machines. This filed is only
exists when the KubeVirt virtual machines are being placed
on a cluster separate from the one hosting the Hosted Control
Plane components. \n The default behavior when Credentials
is not defined is for the KubeVirt VMs to be placed on the
same cluster and namespace as the Hosted Control Plane."
properties:
infraKubeConfigSecret:
description: InfraKubeConfigSecret is a reference to a
secret that contains the kubeconfig for the external
infra cluster that will be used to host the KubeVirt
virtual machines for this cluster.
properties:
key:
type: string
name:
type: string
required:
- key
- name
type: object
infraNamespace:
description: InfraNamespace defines the namespace on the
external infra cluster that is used to host the KubeVirt
virtual machines. This namespace must already exist
before creating the HostedCluster and the kubeconfig
referenced in the InfraKubeConfigSecret must have access
to manage the required resources within this namespace.
type: string
required:
- infraNamespace
type: object
type: object
type: object
replicas:
Expand Down Expand Up @@ -1748,10 +1778,40 @@ spec:
description: CacheName holds the name of the cache DataVolume,
if exists
type: string
remoteNamespace:
description: RemoteNamespace holds the namespace of the remote
infra cluster, if defined
type: string
credentials:
description: "Credentials shows the client credentials used
when creating KubeVirt virtual machines. This filed is only
exists when the KubeVirt virtual machines are being placed
on a cluster separate from the one hosting the Hosted Control
Plane components. \n The default behavior when Credentials
is not defined is for the KubeVirt VMs to be placed on the
same cluster and namespace as the Hosted Control Plane."
properties:
infraKubeConfigSecret:
description: InfraKubeConfigSecret is a reference to a
secret that contains the kubeconfig for the external
infra cluster that will be used to host the KubeVirt
virtual machines for this cluster.
properties:
key:
type: string
name:
type: string
required:
- key
- name
type: object
infraNamespace:
description: InfraNamespace defines the namespace on the
external infra cluster that is used to host the KubeVirt
virtual machines. This namespace must already exist
before creating the HostedCluster and the kubeconfig
referenced in the InfraKubeConfigSecret must have access
to manage the required resources within this namespace.
type: string
required:
- infraNamespace
type: object
type: object
type: object
replicas:
Expand Down
13 changes: 10 additions & 3 deletions docs/content/reference/api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4777,14 +4777,20 @@ string
</tr>
<tr>
<td>
<code>remoteNamespace</code></br>
<code>credentials</code></br>
<em>
string
<a href="#hypershift.openshift.io/v1beta1.KubevirtPlatformCredentials">
KubevirtPlatformCredentials
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>RemoteNamespace holds the namespace of the remote infra cluster, if defined</p>
<p>Credentials shows the client credentials used when creating KubeVirt virtual machines.
This filed is only exists when the KubeVirt virtual machines are being placed
on a cluster separate from the one hosting the Hosted Control Plane components.</p>
<p>The default behavior when Credentials is not defined is for the KubeVirt VMs to be placed on
the same cluster and namespace as the Hosted Control Plane.</p>
</td>
</tr>
</tbody>
Expand Down Expand Up @@ -5061,6 +5067,7 @@ More info: <a href="https://kubernetes.io/docs/concepts/storage/persistent-volum
###KubevirtPlatformCredentials { #hypershift.openshift.io/v1beta1.KubevirtPlatformCredentials }
<p>
(<em>Appears on:</em>
<a href="#hypershift.openshift.io/v1beta1.KubeVirtNodePoolStatus">KubeVirtNodePoolStatus</a>,
<a href="#hypershift.openshift.io/v1beta1.KubevirtPlatformSpec">KubevirtPlatformSpec</a>)
</p>
<p>
Expand Down
80 changes: 72 additions & 8 deletions hack/app-sre/saas_template.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43506,10 +43506,42 @@ objects:
description: CacheName holds the name of the cache DataVolume,
if exists
type: string
remoteNamespace:
description: RemoteNamespace holds the namespace of the
remote infra cluster, if defined
type: string
credentials:
description: "Credentials shows the client credentials used
when creating KubeVirt virtual machines. This filed is
only exists when the KubeVirt virtual machines are being
placed on a cluster separate from the one hosting the
Hosted Control Plane components. \n The default behavior
when Credentials is not defined is for the KubeVirt VMs
to be placed on the same cluster and namespace as the
Hosted Control Plane."
properties:
infraKubeConfigSecret:
description: InfraKubeConfigSecret is a reference to
a secret that contains the kubeconfig for the external
infra cluster that will be used to host the KubeVirt
virtual machines for this cluster.
properties:
key:
type: string
name:
type: string
required:
- key
- name
type: object
infraNamespace:
description: InfraNamespace defines the namespace on
the external infra cluster that is used to host the
KubeVirt virtual machines. This namespace must already
exist before creating the HostedCluster and the kubeconfig
referenced in the InfraKubeConfigSecret must have
access to manage the required resources within this
namespace.
type: string
required:
- infraNamespace
type: object
type: object
type: object
replicas:
Expand Down Expand Up @@ -44387,10 +44419,42 @@ objects:
description: CacheName holds the name of the cache DataVolume,
if exists
type: string
remoteNamespace:
description: RemoteNamespace holds the namespace of the
remote infra cluster, if defined
type: string
credentials:
description: "Credentials shows the client credentials used
when creating KubeVirt virtual machines. This filed is
only exists when the KubeVirt virtual machines are being
placed on a cluster separate from the one hosting the
Hosted Control Plane components. \n The default behavior
when Credentials is not defined is for the KubeVirt VMs
to be placed on the same cluster and namespace as the
Hosted Control Plane."
properties:
infraKubeConfigSecret:
description: InfraKubeConfigSecret is a reference to
a secret that contains the kubeconfig for the external
infra cluster that will be used to host the KubeVirt
virtual machines for this cluster.
properties:
key:
type: string
name:
type: string
required:
- key
- name
type: object
infraNamespace:
description: InfraNamespace defines the namespace on
the external infra cluster that is used to host the
KubeVirt virtual machines. This namespace must already
exist before creating the HostedCluster and the kubeconfig
referenced in the InfraKubeConfigSecret must have
access to manage the required resources within this
namespace.
type: string
required:
- infraNamespace
type: object
type: object
type: object
replicas:
Expand Down
4 changes: 2 additions & 2 deletions hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3467,7 +3467,7 @@ func (r *HostedClusterReconciler) reconcileNetworkPolicies(ctx context.Context,
}); err != nil {
return fmt.Errorf("failed to reconcile virt launcher policy: %w", err)
}
kvInfraCluster, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, r.Client, hcluster.Spec.InfraID, hcluster, hcp.Namespace)
kvInfraCluster, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, r.Client, hcluster.Spec.InfraID, hcluster.Spec.Platform.Kubevirt.Credentials, hcp.Namespace, hcluster.Namespace)
if err != nil {
return err
}
Expand Down Expand Up @@ -4606,7 +4606,7 @@ func (r *HostedClusterReconciler) reconcileKubevirtPlatformDefaultSettings(ctx c
// auto generate the basedomain by retrieving the default ingress *.apps dns.
if hc.Spec.Platform.Kubevirt.BaseDomainPassthrough != nil && *hc.Spec.Platform.Kubevirt.BaseDomainPassthrough {
if hc.Spec.DNS.BaseDomain == "" {
kvInfraCluster, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, r.Client, hc.Spec.InfraID, hc, hc.Namespace)
kvInfraCluster, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, r.Client, hc.Spec.InfraID, hc.Spec.Platform.Kubevirt.Credentials, hc.Namespace, hc.Namespace)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3051,7 +3051,7 @@ func newKVInfraMapMock(objects []crclient.Object) kvinfra.KubevirtInfraClientMap
}
}

func (k *kubevirtInfraClientMapMock) DiscoverKubevirtClusterClient(_ context.Context, _ crclient.Client, _ string, _ *hyperv1.HostedCluster, _ string) (*kvinfra.KubevirtInfraClient, error) {
func (k *kubevirtInfraClientMapMock) DiscoverKubevirtClusterClient(_ context.Context, _ crclient.Client, _ string, _ *hyperv1.KubevirtPlatformCredentials, _ string, _ string) (*kvinfra.KubevirtInfraClient, error) {
return k.cluster, nil
}

Expand Down
26 changes: 15 additions & 11 deletions hypershift-operator/controllers/nodepool/nodepool_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho
nodePool.Status.Platform.KubeVirt = &hyperv1.KubeVirtNodePoolStatus{}
}

nodePool.Status.Platform.KubeVirt.RemoteNamespace = infraNS
nodePool.Status.Platform.KubeVirt.Credentials = hcluster.Spec.Platform.Kubevirt.Credentials.DeepCopy()
}
kubevirtBootImage, err = kubevirt.GetImage(nodePool, releaseImage, infraNS)
if err != nil {
Expand All @@ -374,7 +374,7 @@ func (r *NodePoolReconciler) reconcile(ctx context.Context, hcluster *hyperv1.Ho
})

uid := string(nodePool.GetUID())
kvInfraClient, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, r.Client, uid, hcluster, controlPlaneNamespace)
kvInfraClient, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, r.Client, uid, hcluster.Spec.Platform.Kubevirt.Credentials, controlPlaneNamespace, hcluster.GetNamespace())
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get KubeVirt external infra-cluster: %w", err)
}
Expand Down Expand Up @@ -1257,25 +1257,29 @@ func (r *NodePoolReconciler) delete(ctx context.Context, nodePool *hyperv1.NodeP
return err
}

r.KubevirtInfraClients.Delete(string(nodePool.GetUID()))

return nil
}

func (r *NodePoolReconciler) deleteKubeVirtCache(ctx context.Context, nodePool *hyperv1.NodePool, controlPlaneNamespace string) error {
if nodePool.Status.Platform != nil {
if nodePool.Status.Platform.KubeVirt != nil {
if cacheName := nodePool.Status.Platform.KubeVirt.CacheName; len(cacheName) > 0 {
ns := controlPlaneNamespace
uid := string(nodePool.GetUID())
cl, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, r.Client, uid, nodePool.Status.Platform.KubeVirt.Credentials, controlPlaneNamespace, nodePool.GetNamespace())
if err != nil {
return fmt.Errorf("failed to get KubeVirt external infra-cluster: %w", err)
}

if len(nodePool.Status.Platform.KubeVirt.RemoteNamespace) > 0 {
ns = nodePool.Status.Platform.KubeVirt.RemoteNamespace
ns := controlPlaneNamespace
if nodePool.Status.Platform.KubeVirt.Credentials != nil && len(nodePool.Status.Platform.KubeVirt.Credentials.InfraNamespace) > 0 {
ns = nodePool.Status.Platform.KubeVirt.Credentials.InfraNamespace
}

if cl := r.KubevirtInfraClients.GetClient(string(nodePool.GetUID())); cl != nil {
err := kubevirt.DeleteCache(ctx, cl, cacheName, ns)
if err != nil {
return err
}
r.KubevirtInfraClients.Delete(string(nodePool.GetUID()))
err = kubevirt.DeleteCache(ctx, cl, cacheName, ns)
if err != nil {
return err
}
}
}
Expand Down
Loading

0 comments on commit 8cda39e

Please sign in to comment.