From a318b1cb03234c584cd4690223d69293b7d05581 Mon Sep 17 00:00:00 2001 From: Caleb Xu Date: Fri, 4 Sep 2026 12:48:15 -0400 Subject: [PATCH 1/5] chore(config): use the default manager image reference Signed-off-by: Caleb Xu Assisted-by: OpenCode (GPT-5.6 Terra) --- config/manager/kustomization.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 024e5ed..8302895 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: image-registry.openshift-image-registry.svc:5000/guestcluster-operator-system/guestcluster-operator-controller - newTag: latest + newName: example.com/guestcluster-operator + newTag: v0.0.1 From bc3f80a75aee4969970266befd98c36ad4d5160d Mon Sep 17 00:00:00 2001 From: Caleb Xu Date: Fri, 4 Sep 2026 12:48:22 -0400 Subject: [PATCH 2/5] feat(crc): persist instance identity credentials Signed-off-by: Caleb Xu Assisted-by: OpenCode (GPT-5.6 Terra) --- cmd/crc-agent/cluster.go | 30 ++-- cmd/crc-agent/guest.go | 61 ++----- cmd/crc-agent/main.go | 130 ++++++++++---- cmd/crc-agent/sshrunner.go | 18 ++ cmd/crc-agent/vmiwatch.go | 163 ++++++++++++++++++ cmd/crc-agent/vmiwatch_test.go | 166 ++++++++++++++++++ internal/resources/crcidentity.go | 229 +++++++++++++++++++++++++ internal/resources/crcidentity_test.go | 48 ++++++ internal/resources/kascert.go | 6 +- 9 files changed, 765 insertions(+), 86 deletions(-) create mode 100644 cmd/crc-agent/vmiwatch.go create mode 100644 cmd/crc-agent/vmiwatch_test.go create mode 100644 internal/resources/crcidentity.go create mode 100644 internal/resources/crcidentity_test.go diff --git a/cmd/crc-agent/cluster.go b/cmd/crc-agent/cluster.go index 2793f57..cd9dc33 100644 --- a/cmd/crc-agent/cluster.go +++ b/cmd/crc-agent/cluster.go @@ -32,6 +32,7 @@ limitations under the License. package main import ( + "bytes" "context" "crypto/rand" "encoding/base64" @@ -47,6 +48,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" + + "github.com/caxu-rh/guestcluster-operator/internal/resources" ) const ( @@ -104,6 +107,9 @@ type clusterFixupConfig struct { // internal/resources.BuildCRCAPIRoute) used for the external API // serving cert + apiserver namedCertificate patch. APIHostname string + // Identity is the management-side certificate material that remains stable + // when the backing VMI is replaced. + Identity resources.CRCIdentity } // ClusterFixupResult is what a successful RunClusterFixups call returns. @@ -168,7 +174,9 @@ func RunClusterFixups( // 5. External API serving cert + apiserver namedCertificate patch log.Info("cluster: applying external API patches", "apiHostname", cfg.APIHostname) - externalAPICACertPEM, err := applyExternalAPIPatches(ctx, clients, cfg.APIHostname) + externalAPICACertPEM, err := applyExternalAPIPatches( + ctx, clients, cfg.APIHostname, cfg.Identity.ServingCert, cfg.Identity.ServingPrivateKey, + ) if err != nil { return nil, fmt.Errorf("external API patches: %w", err) } @@ -326,8 +334,8 @@ func updatePasswords(ctx context.Context, clients *GuestClients, kubeadminPass, // External API access (cert + apiserver namedCertificate) // --------------------------------------------------------------------------- -// applyExternalAPIPatches generates the self-signed serving certificate for -// the externally routable API hostname (the management cluster's +// applyExternalAPIPatches installs the stable serving certificate for the +// externally routable API hostname (the management cluster's // passthrough Route; see internal/resources.BuildCRCAPIRoute). It publishes // the certificate as a TLS secret (see externalAPICertSecretName), and it // patches the apiserver config so the API server presents that @@ -343,11 +351,9 @@ func updatePasswords(ctx context.Context, clients *GuestClients, kubeadminPass, // not the guest's web console, apps, or image registry. The guest's own // VMI network is not routable from outside the management cluster for // arbitrary wildcard hostnames the way a single API hostname is. -func applyExternalAPIPatches(ctx context.Context, clients *GuestClients, apiHostname string) ([]byte, error) { - certPEM, keyPEM, err := ExternalAPIServingCert(apiHostname) - if err != nil { - return nil, fmt.Errorf("generating external API serving cert: %w", err) - } +func applyExternalAPIPatches( + ctx context.Context, clients *GuestClients, apiHostname string, certPEM, keyPEM []byte, +) ([]byte, error) { if err := createOrUpdateTLSSecret( ctx, clients, externalAPICertSecretNamespace, externalAPICertSecretName, certPEM, keyPEM, ); err != nil { @@ -375,8 +381,12 @@ func createOrUpdateTLSSecret(ctx context.Context, clients *GuestClients, ns, nam if getErr != nil { return getErr } - existing.Data = sec.Data - existing.Type = sec.Type + if existing.Type == sec.Type && + bytes.Equal(existing.Data[corev1.TLSCertKey], cert) && + bytes.Equal(existing.Data[corev1.TLSPrivateKeyKey], key) { + return nil + } + existing.Data, existing.Type = sec.Data, sec.Type _, err = clients.Core.CoreV1().Secrets(ns).Update(ctx, existing, metav1.UpdateOptions{}) } return err diff --git a/cmd/crc-agent/guest.go b/cmd/crc-agent/guest.go index 78f9b77..6b73277 100644 --- a/cmd/crc-agent/guest.go +++ b/cmd/crc-agent/guest.go @@ -24,8 +24,8 @@ limitations under the License. // 1. dnsmasq must start so api.crc.testing resolves inside the VM // (needed for `oc` commands issued over SSH). // 2. The kubelet must start so the API server comes up. -// 3. bootstrapCA must regenerate the CA and replace the bundle's stale -// admin client cert with a new one. This is the only step that still +// 3. bootstrapCA installs the stable CA and admin client certificate. This is +// the only step that still // runs oc on the guest over SSH. Typed client auth would create a // circular dependency: it needs a valid cert to connect, but it needs // to connect to replace the cert. @@ -46,8 +46,6 @@ import ( "bytes" "crypto/ed25519" "crypto/rand" - "crypto/rsa" - "crypto/x509" "encoding/base64" "encoding/json" "fmt" @@ -57,6 +55,8 @@ import ( gossh "golang.org/x/crypto/ssh" k8syaml "sigs.k8s.io/yaml" + + "github.com/caxu-rh/guestcluster-operator/internal/resources" ) // generateEd25519Key generates a fresh ed25519 keypair. @@ -102,28 +102,19 @@ type guestResult struct { // the new admin client cert injected. The SSH-tunneled typed client uses // this as its TLS credential. AdminKubeconfigPEM []byte - // CACert is the self-signed CA that bootstrapCA generates. The API server - // trusts this CA for verifying client certificates through the - // admin-kubeconfig-client-ca configmap (it signs ClientCertPEM below). - // It does not sign the API server's own TLS serving certificate. See - // ServerCAPEM for that. - CACert *x509.Certificate - // CAKey is the matching private key, kept for later use if needed. - CAKey *rsa.PrivateKey - // ClientCertPEM and ClientKeyPEM are the admin client cert and key PEM - // bytes, signed by CACert. + // ClientCertPEM and ClientKeyPEM are the mounted admin client credential. ClientCertPEM []byte ClientKeyPEM []byte // ServerCAPEM is the CA bundle that verifies the API server's own TLS // serving certificate for the internal api.crc.testing SNI. bootstrapCA // extracts it from the bundle kubeconfig's // clusters[0].cluster.certificate-authority-data field, which it never - // modifies. This is a different trust root than CACert: it is whatever + // modifies. This is separate from the mounted client CA: it is whatever // the CRC bundle was built with (for example kube-apiserver-lb-signer // and related certs), not something crc-agent generates. Typed clients // connecting to api.crc.testing (clusterclient.go) must use this as - // their CAData, not CACert. Conflating the two produces "certificate - // signed by unknown authority", because CACert never signed the + // their CAData, not the client CA. Conflating the two produces "certificate + // signed by unknown authority", because the client CA never signed the // server's serving cert. ServerCAPEM []byte } @@ -154,7 +145,7 @@ nameserver {{ .IP }} // 2. Rewrite /etc/resolv.conf so the VM resolves *.crc.testing. // 3. Generate a new ed25519 SSH keypair and swap it onto authorized_keys. // 4. Start the kubelet. -// 5. Regenerate the admin CA and client cert, then patch the cluster +// 5. Install the stable admin CA and client cert, then patch the cluster // (the CA bootstrap step; this uses oc on the guest for this one step). // 6. Return the new kubeconfig bytes and crypto material for the // typed-client stage. @@ -196,9 +187,9 @@ func RunGuestFixups(runner *Runner, cfg config, log logrLike) (*guestResult, err return nil, fmt.Errorf("start kubelet: %w", err) } - // 5. CA bootstrap: generate new CA+client cert, patch the cluster - log.Info("guest: regenerating admin CA and client cert (bootstrap)") - res, err := bootstrapCA(runner) + // 5. CA bootstrap: install the stable CA+client cert and patch the cluster. + log.Info("guest: installing stable admin CA and client cert") + res, err := bootstrapCA(runner, cfg.Identity) if err != nil { return nil, fmt.Errorf("bootstrap CA: %w", err) } @@ -340,10 +331,8 @@ func startKubelet(runner *Runner) error { return nil } -// bootstrapCA is the CA-regen bootstrap step: -// 1. Generate a self-signed CA (crypto/x509). -// 2. Mint a system:admin / system:masters client cert. -// 3. Read the bundle's /opt/kubeconfig and splice in the new client cert. +// bootstrapCA installs the management-side identity into the guest: +// 1. Read the bundle's /opt/kubeconfig and splice in the stable client cert. // 4. Run `oc patch configmap admin-kubeconfig-client-ca` on the guest over // SSH so the API server trusts the new CA. This is the one intentional // oc-on-guest exception, matching crc's own approach: the API server @@ -352,17 +341,7 @@ func startKubelet(runner *Runner) error { // // Ported from crc pkg/crc/machine/start.go updateKubeconfig and // pkg/crc/cluster/cluster.go EnsureGeneratedClientCAPresentInTheCluster. -func bootstrapCA(runner *Runner) (*guestResult, error) { - // Generate CA + client cert. - caKey, caCert, err := SelfSignedCA() - if err != nil { - return nil, err - } - clientCertPEM, clientKeyPEM, err := ClientCertificate(caKey, caCert) - if err != nil { - return nil, err - } - caPEM := CAPem(caCert) +func bootstrapCA(runner *Runner, identity resources.CRCIdentity) (*guestResult, error) { // Read the bundle's admin kubeconfig from the guest, with retries. The // caller started the kubelet a moment ago, and the kubelet may not have @@ -382,7 +361,7 @@ func bootstrapCA(runner *Runner) (*guestResult, error) { // Splice in the new client cert+key. patchedKubeconfig, err := spliceClientCertIntoKubeconfig( - []byte(bundleKubeconfigYAML), clientCertPEM, clientKeyPEM) + []byte(bundleKubeconfigYAML), identity.ClientCert, identity.ClientPrivateKey) if err != nil { return nil, fmt.Errorf("splicing client cert into kubeconfig: %w", err) } @@ -390,7 +369,7 @@ func bootstrapCA(runner *Runner) (*guestResult, error) { // Patch admin-kubeconfig-client-ca configmap via oc on the guest. // This is the only oc-on-guest call; it must happen BEFORE typed clients // try to authenticate, so the API server trusts our new CA. - caPEMJSON, _ := json.Marshal(string(caPEM)) + caPEMJSON, _ := json.Marshal(string(identity.ClientCA)) patchCmd := fmt.Sprintf( `oc --kubeconfig /opt/kubeconfig patch configmap admin-kubeconfig-client-ca `+ `-n openshift-config --patch '{"data":{"ca-bundle.crt":%s}}'`, @@ -429,11 +408,9 @@ func bootstrapCA(runner *Runner) (*guestResult, error) { return &guestResult{ AdminKubeconfigPEM: patchedKubeconfig, - CACert: caCert, - CAKey: caKey, ServerCAPEM: serverCAPEM, - ClientCertPEM: clientCertPEM, - ClientKeyPEM: clientKeyPEM, + ClientCertPEM: identity.ClientCert, + ClientKeyPEM: identity.ClientPrivateKey, }, nil } diff --git a/cmd/crc-agent/main.go b/cmd/crc-agent/main.go index 4d9411c..5ce1934 100644 --- a/cmd/crc-agent/main.go +++ b/cmd/crc-agent/main.go @@ -43,7 +43,7 @@ limitations under the License. // - Approve pending kubelet CSRs. // - Inject the real pull secret. // - Update kubeadmin + developer passwords (bcrypt in-process, no podman). -// - Generate a self-signed serving cert for cfg.APIHostname (the +// - Install the stable serving cert for cfg.APIHostname (the // management-cluster passthrough Route host the ClusterInstance // controller already provisioned, see // internal/resources.BuildCRCAPIRoute) and patch the apiserver's @@ -52,18 +52,16 @@ limitations under the License. // - Read the OpenShift version from ClusterVersion. // 5. Produce a routable admin kubeconfig (server rewritten to // https://) and publish it together with the OCP -// version as a Secret named "-crc-raw-kubeconfig" in the +// version as a VMI-specific Secret in the // ClusterInstance's namespace (keys "kubeconfig" and "ocpVersion"). // This Secret is the only contract between crc-agent and the // ClusterInstance controller. ensureCRCBacking (clusterinstance_crc.go) // reads this Secret and never SSHes anywhere itself. // -// crc-agent is deployed as a Kubernetes Job, one per ClusterInstance. The -// controller recreates the Job on every recycle, so a fresh run always -// accompanies a fresh VM boot. This recreation also refreshes the bundle's -// ~30-day-limited certificates. The Job runs to completion. Success means the -// raw kubeconfig Secret now exists. Failure exits non-zero, and the Job's -// BackoffLimit governs retries. +// crc-agent is deployed as a Kubernetes Job, one per ClusterInstance VMI. The +// controller creates a new, VMI-scoped Job after a VMI replacement. The Job runs to completion. +// Success means the raw kubeconfig Secret now exists. Failure exits non-zero, +// and the Job's BackoffLimit governs retries. // // The crc-agent container image needs only the following host tools: // - curl, tar (zstd), jq, sha256sum, sed, coreutils, qemu-img: for the @@ -77,15 +75,18 @@ package main import ( "context" "encoding/base64" + "errors" "flag" "fmt" "os" + "reflect" "time" gossh "golang.org/x/crypto/ssh" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" @@ -102,22 +103,24 @@ import ( // spec can set it (see internal/resources.BuildCRCAgentJob) without any // additional CRDs of its own. type config struct { - Namespace string // ClusterInstance/Secret namespace - InstanceName string // ClusterInstance name; also the CRC VM name (resources.VMName) - SSHHost string // CRC VM's reachable IP/host (populated from VMI status by the controller) - SSHPort int - SSHUser string - SSHKeyPath string // path to the CRC bundle's SSH private key, mounted into the container + Namespace string // ClusterInstance/Secret namespace + InstanceName string // ClusterInstance name; also the CRC VM name (resources.VMName) + SSHHost string // CRC VM's reachable IP/host (populated from VMI status by the controller) + ExpectedVMIUID string // UID of the VMI this Job is allowed to configure + SSHPort int + SSHUser string + SSHKeyPath string // path to the CRC bundle's SSH private key, mounted into the container // APIHostname is the externally-routable hostname of the management // cluster's passthrough Route fronting the guest API server (see // internal/resources.BuildCRCAPIRoute), populated by the - // ClusterInstance controller via CRC_API_HOSTNAME. Used to mint the - // guest API server's external-facing serving cert and to rewrite the - // published kubeconfig's server URL. + // ClusterInstance controller via CRC_API_HOSTNAME. Used to validate the + // mounted identity and rewrite the published kubeconfig's server URL. APIHostname string PullSecretPath string // path to the real pull secret file, mounted into the container + Identity resources.CRCIdentity + IdentityPath string SSHReadyTimeout time.Duration // how long to wait for the VM's sshd to accept connections SSHRetryInterval time.Duration // how often to retry the SSH-reachability check @@ -128,10 +131,12 @@ func configFromEnv() config { Namespace: os.Getenv("INSTANCE_NAMESPACE"), InstanceName: os.Getenv("INSTANCE_NAME"), SSHHost: os.Getenv("CRC_SSH_HOST"), + ExpectedVMIUID: os.Getenv("CRC_VMI_UID"), SSHUser: envDefault("CRC_SSH_USER", "core"), SSHKeyPath: envDefault("CRC_SSH_KEY_PATH", resources.CRCAgentSSHKeyPath()), APIHostname: os.Getenv(resources.CRCAPIHostnameEnvVar), PullSecretPath: envDefault("PULL_SECRET_PATH", resources.CRCAgentPullSecretPath()), + IdentityPath: envDefault("CRC_IDENTITY_PATH", "/etc/crc-agent/identity"), } c.SSHPort = 22 c.SSHReadyTimeout = 5 * time.Minute @@ -163,10 +168,11 @@ func main() { flag.StringVar(&cfg.InstanceName, "instance-name", cfg.InstanceName, "ClusterInstance name this agent serves") flag.StringVar(&cfg.Namespace, "namespace", cfg.Namespace, "Namespace of the ClusterInstance/Secret") flag.StringVar(&cfg.SSHHost, "ssh-host", cfg.SSHHost, "SSH-reachable host/IP of the CRC VM") + flag.StringVar(&cfg.ExpectedVMIUID, "vmi-uid", cfg.ExpectedVMIUID, "UID of the CRC VMI") flag.Parse() - if cfg.InstanceName == "" || cfg.Namespace == "" || cfg.SSHHost == "" { - const missingConfigMsg = "INSTANCE_NAME, INSTANCE_NAMESPACE and CRC_SSH_HOST " + + if cfg.InstanceName == "" || cfg.Namespace == "" || cfg.SSHHost == "" || cfg.ExpectedVMIUID == "" { + const missingConfigMsg = "INSTANCE_NAME, INSTANCE_NAMESPACE, CRC_SSH_HOST and CRC_VMI_UID " + "(or their --flag equivalents) are required" log.Error(fmt.Errorf("missing required configuration"), missingConfigMsg) os.Exit(1) @@ -177,24 +183,59 @@ func main() { log.Error(err, "mounted bundle SSH key failed pre-flight validation") os.Exit(1) } + identity, err := loadCRCIdentity(cfg.IdentityPath, cfg.APIHostname) + if err != nil { + log.Error(err, "mounted CRC identity failed pre-flight validation") + os.Exit(1) + } + cfg.Identity = identity - kc, err := kubeClient() + restCfg, err := kubeRESTConfig() + if err != nil { + log.Error(err, "unable to build Kubernetes client") + os.Exit(1) + } + kc, err := kubernetes.NewForConfig(restCfg) if err != nil { log.Error(err, "unable to build Kubernetes client") os.Exit(1) } + vmiClient, err := dynamic.NewForConfig(restCfg) + if err != nil { + log.Error(err, "unable to build KubeVirt client") + os.Exit(1) + } ctx := ctrl.SetupSignalHandler() + watchedCtx, stopVMIWatch, err := monitorCRCVMILifecycle(ctx, vmiClient, cfg, log) + if err != nil { + log.Error(err, "unable to monitor CRC VirtualMachineInstance") + os.Exit(1) + } + defer stopVMIWatch() - log.Info("starting crc-agent", "instance", cfg.InstanceName, "namespace", cfg.Namespace, "sshHost", cfg.SSHHost) + log.Info("starting crc-agent", "instance", cfg.InstanceName, "namespace", cfg.Namespace, + "sshHost", cfg.SSHHost, "vmiUid", cfg.ExpectedVMIUID) - info, err := fetchClusterInfo(ctx, log, cfg, bundleSigner) + info, err := fetchClusterInfo(watchedCtx, log, cfg, bundleSigner) if err != nil { + if errors.Is(context.Cause(watchedCtx), errCRCVMINoLongerCurrent) { + log.Info("CRC VirtualMachineInstance is no longer current; ending agent run") + return + } log.Error(err, "failed to bring up CRC cluster") os.Exit(1) } - if err := publishRawKubeconfig(ctx, kc, cfg, info); err != nil { + if err := ensureExpectedCRCVMIRunning(ctx, vmiClient.Resource(crcVMIGVR).Namespace(cfg.Namespace), cfg); err != nil { + if errors.Is(err, errCRCVMINoLongerCurrent) { + log.Info("CRC VirtualMachineInstance is no longer current; not publishing handoff") + return + } + log.Error(err, "unable to verify CRC VirtualMachineInstance before publishing handoff") + os.Exit(1) + } + if err := publishRawKubeconfig(watchedCtx, kc, cfg, info); err != nil { log.Error(err, "failed to publish raw kubeconfig secret") os.Exit(1) } @@ -202,10 +243,7 @@ func main() { log.Info("published raw kubeconfig, crc-agent run complete", "ocpVersion", info.OCPVersion) } -// kubeClient builds a client-go Clientset, preferring in-cluster config (the normal -// deployment mode, as a Job Pod in the mgmt cluster) and falling back to KUBECONFIG -// for local development/testing of the agent binary itself. -func kubeClient() (*kubernetes.Clientset, error) { +func kubeRESTConfig() (*rest.Config, error) { restCfg, err := rest.InClusterConfig() if err != nil { kubeconfig := os.Getenv("KUBECONFIG") @@ -217,7 +255,7 @@ func kubeClient() (*kubernetes.Clientset, error) { return nil, fmt.Errorf("building config from KUBECONFIG: %w", err) } } - return kubernetes.NewForConfig(restCfg) + return restCfg, nil } // fetchClusterInfo is the main orchestration function. It performs these steps: @@ -241,9 +279,11 @@ func fetchClusterInfo(ctx context.Context, log logrLike, cfg config, bundleSigne if err != nil { return nil, fmt.Errorf("SSH connect (bundle key): %w", err) } + stopClosingRunner := closeRunnerOnCancellation(ctx, runner) log.Info("running guest-side fixups") guestRes, err := RunGuestFixups(runner, cfg, log) + stopClosingRunner() if err != nil { _ = runner.Close() return nil, fmt.Errorf("guest fixups: %w", err) @@ -256,11 +296,13 @@ func fetchClusterInfo(ctx context.Context, log logrLike, cfg config, bundleSigne if err != nil { return nil, fmt.Errorf("SSH reconnect (new key): %w", err) } + stopClosingRunner2 := closeRunnerOnCancellation(ctx, runner2) + defer stopClosingRunner2() defer func() { _ = runner2.Close() }() // 4. Build typed guest clients (routed through the SSH tunnel). // The CA here must be guestRes.ServerCAPEM, the bundle's own - // server-serving-cert CA. It must not be CAPem(guestRes.CACert): that CA + // server-serving-cert CA. It must not be the mounted client CA: that CA // only signs the client cert (trusted through admin-kubeconfig-client-ca) // and never signed the API server's own TLS certificate for the // api.crc.testing SNI. Using that CA here produces "certificate signed @@ -284,6 +326,7 @@ func fetchClusterInfo(ctx context.Context, log logrLike, cfg config, bundleSigne fixupCfg := clusterFixupConfig{ PullSecretJSON: pullSecretJSON, APIHostname: cfg.APIHostname, + Identity: cfg.Identity, } log.Info("running cluster-level fixups") @@ -374,11 +417,30 @@ func loadSigner(path string) (gossh.Signer, error) { return signer, nil } +func loadCRCIdentity(path, hostname string) (resources.CRCIdentity, error) { + keys := []string{ + resources.CRCIdentityClientCAKey, + resources.CRCIdentityClientCertKey, + resources.CRCIdentityClientPrivateKey, + resources.CRCIdentityServingCertKey, + resources.CRCIdentityServingPrivateKey, + } + data := make(map[string][]byte, len(keys)) + for _, key := range keys { + value, err := os.ReadFile(path + "/" + key) + if err != nil { + return resources.CRCIdentity{}, fmt.Errorf("reading identity key %q: %w", key, err) + } + data[key] = value + } + return resources.CRCIdentityFromSecretData(data, hostname) +} + // publishRawKubeconfig creates or updates the raw kubeconfig Secret that the // ClusterInstance controller's ensureCRCBacking reads (see -// internal/resources.RawKubeconfigSecretName/KubeconfigSecretKey/OCPVersionSecretKey). +// internal/resources.RawKubeconfigSecretNameForVMI/KubeconfigSecretKey/OCPVersionSecretKey). func publishRawKubeconfig(ctx context.Context, kc *kubernetes.Clientset, cfg config, info *clusterInfo) error { - name := resources.RawKubeconfigSecretName(cfg.InstanceName) + name := resources.RawKubeconfigSecretNameForVMI(cfg.InstanceName, cfg.ExpectedVMIUID) secretsClient := kc.CoreV1().Secrets(cfg.Namespace) secret := &corev1.Secret{ @@ -394,6 +456,7 @@ func publishRawKubeconfig(ctx context.Context, kc *kubernetes.Clientset, cfg con Data: map[string][]byte{ resources.KubeconfigSecretKey: info.Kubeconfig, resources.OCPVersionSecretKey: []byte(info.OCPVersion), + resources.VMIUIDSecretKey: []byte(cfg.ExpectedVMIUID), }, } @@ -403,7 +466,10 @@ func publishRawKubeconfig(ctx context.Context, kc *kubernetes.Clientset, cfg con if getErr != nil { return getErr } - existing.Data = secret.Data + if reflect.DeepEqual(existing.Data, secret.Data) && existing.Type == secret.Type { + return nil + } + existing.Data, existing.Type = secret.Data, secret.Type _, err = secretsClient.Update(ctx, existing, metav1.UpdateOptions{}) } return err diff --git a/cmd/crc-agent/sshrunner.go b/cmd/crc-agent/sshrunner.go index 343637d..336dd64 100644 --- a/cmd/crc-agent/sshrunner.go +++ b/cmd/crc-agent/sshrunner.go @@ -40,6 +40,7 @@ import ( "encoding/base64" "fmt" "net" + "sync" "time" gossh "golang.org/x/crypto/ssh" @@ -75,6 +76,23 @@ func (r *Runner) Close() error { return r.client.Close() } +// closeRunnerOnCancellation closes SSH sessions that cannot otherwise observe +// context cancellation, including a blocked Session.Run call. +func closeRunnerOnCancellation(ctx context.Context, runner *Runner) func() { + done := make(chan struct{}) + var once sync.Once + go func() { + select { + case <-ctx.Done(): + _ = runner.Close() + case <-done: + } + }() + return func() { + once.Do(func() { close(done) }) + } +} + // Run executes cmd on the guest and returns combined stdout+stderr. // A non-zero exit code is returned as an error. func (r *Runner) Run(cmd string) (string, error) { diff --git a/cmd/crc-agent/vmiwatch.go b/cmd/crc-agent/vmiwatch.go new file mode 100644 index 0000000..fe557c9 --- /dev/null +++ b/cmd/crc-agent/vmiwatch.go @@ -0,0 +1,163 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + "errors" + "fmt" + "time" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + kubevirtv1 "kubevirt.io/api/core/v1" +) + +var ( + crcVMIGVR = schema.GroupVersionResource{ + Group: "kubevirt.io", Version: "v1", Resource: "virtualmachineinstances", + } + errCRCVMINoLongerCurrent = errors.New("CRC VirtualMachineInstance is no longer current") +) + +const vmiWatchRetryInterval = time.Second + +// monitorCRCVMILifecycle verifies the VMI before guest changes start, then +// cancels the returned context if that VMI is deleted or replaced. It relists +// before each watch to recover from closed or expired watch streams. +func monitorCRCVMILifecycle( + ctx context.Context, client dynamic.Interface, cfg config, log logrLike, +) (context.Context, context.CancelFunc, error) { + vmis := client.Resource(crcVMIGVR).Namespace(cfg.Namespace) + if err := ensureExpectedCRCVMIRunning(ctx, vmis, cfg); err != nil { + return nil, nil, err + } + + watchCtx, cancel := context.WithCancelCause(ctx) + go func() { + for { + vmi, err := getExpectedCRCVMI(watchCtx, vmis, cfg) + if err != nil { + if errors.Is(err, errCRCVMINoLongerCurrent) { + cancel(err) + return + } + log.Info("unable to list CRC VirtualMachineInstance; retrying watch", "error", err.Error()) + if !waitForVMIWatchRetry(watchCtx) { + return + } + continue + } + + w, err := vmis.Watch(watchCtx, metav1.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("metadata.name", cfg.InstanceName).String(), + ResourceVersion: vmi.GetResourceVersion(), + AllowWatchBookmarks: true, + }) + if err != nil { + log.Info("unable to watch CRC VirtualMachineInstance; retrying", "error", err.Error()) + if !waitForVMIWatchRetry(watchCtx) { + return + } + continue + } + + if err := waitForCRCVMITermination(watchCtx, w, cfg); err != nil { + w.Stop() + cancel(err) + return + } + w.Stop() + if !waitForVMIWatchRetry(watchCtx) { + return + } + } + }() + + return watchCtx, func() { cancel(nil) }, nil +} + +func ensureExpectedCRCVMIRunning(ctx context.Context, vmis dynamic.ResourceInterface, cfg config) error { + _, err := getExpectedCRCVMI(ctx, vmis, cfg) + return err +} + +func getExpectedCRCVMI( + ctx context.Context, vmis dynamic.ResourceInterface, cfg config, +) (*unstructured.Unstructured, error) { + vmi, err := vmis.Get(ctx, cfg.InstanceName, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return nil, fmt.Errorf("%w: %s/%s was deleted", errCRCVMINoLongerCurrent, cfg.Namespace, cfg.InstanceName) + } + if err != nil { + return nil, err + } + if string(vmi.GetUID()) != cfg.ExpectedVMIUID { + return nil, fmt.Errorf("%w: %s/%s UID changed from %s to %s", + errCRCVMINoLongerCurrent, cfg.Namespace, cfg.InstanceName, cfg.ExpectedVMIUID, vmi.GetUID()) + } + phase, _, _ := unstructured.NestedString(vmi.Object, "status", "phase") + if phase != string(kubevirtv1.Running) { + return nil, fmt.Errorf("%w: %s/%s is not running", errCRCVMINoLongerCurrent, cfg.Namespace, cfg.InstanceName) + } + return vmi, nil +} + +func waitForCRCVMITermination(ctx context.Context, w watch.Interface, cfg config) error { + for { + select { + case <-ctx.Done(): + return nil + case event, ok := <-w.ResultChan(): + if !ok { + return nil + } + vmi, ok := event.Object.(*unstructured.Unstructured) + if !ok { + continue + } + switch event.Type { + case watch.Deleted: + return fmt.Errorf("%w: %s/%s was deleted or replaced", errCRCVMINoLongerCurrent, cfg.Namespace, cfg.InstanceName) + case watch.Added, watch.Modified: + if string(vmi.GetUID()) != cfg.ExpectedVMIUID { + return fmt.Errorf("%w: %s/%s was deleted or replaced", errCRCVMINoLongerCurrent, cfg.Namespace, cfg.InstanceName) + } + phase, _, _ := unstructured.NestedString(vmi.Object, "status", "phase") + if phase != string(kubevirtv1.Running) { + return fmt.Errorf("%w: %s/%s is not running", errCRCVMINoLongerCurrent, cfg.Namespace, cfg.InstanceName) + } + } + } + } +} + +func waitForVMIWatchRetry(ctx context.Context) bool { + timer := time.NewTimer(vmiWatchRetryInterval) + defer timer.Stop() + select { + case <-ctx.Done(): + return false + case <-timer.C: + return true + } +} diff --git a/cmd/crc-agent/vmiwatch_test.go b/cmd/crc-agent/vmiwatch_test.go new file mode 100644 index 0000000..96829c9 --- /dev/null +++ b/cmd/crc-agent/vmiwatch_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + "errors" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic/fake" +) + +const ( + testVMINamespace = "test" + testVMIName = "crc" + testVMIUID = "vmi-uid" +) + +func TestMonitorCRCVMILifecycleCancelsWhenVMIIsDeleted(t *testing.T) { + client := newVMIDynamicClient(t, testVMIUID) + cfg := config{Namespace: testVMINamespace, InstanceName: testVMIName, ExpectedVMIUID: testVMIUID} + ctx, stop, err := monitorCRCVMILifecycle(context.Background(), client, cfg, discardLog{}) + if err != nil { + t.Fatalf("monitorCRCVMILifecycle: %v", err) + } + defer stop() + + vmis := client.Resource(crcVMIGVR).Namespace(cfg.Namespace) + if err := vmis.Delete(context.Background(), cfg.InstanceName, metav1.DeleteOptions{}); err != nil { + t.Fatalf("deleting VMI: %v", err) + } + + select { + case <-ctx.Done(): + if !errors.Is(context.Cause(ctx), errCRCVMINoLongerCurrent) { + t.Fatalf("context cause = %v, want VMI lifecycle error", context.Cause(ctx)) + } + case <-time.After(5 * time.Second): + t.Fatal("VMI deletion did not cancel the agent context") + } +} + +func TestEnsureExpectedCRCVMIRunningRejectsReplacement(t *testing.T) { + client := newVMIDynamicClient(t, "new-vmi-uid") + cfg := config{Namespace: testVMINamespace, InstanceName: testVMIName, ExpectedVMIUID: "old-vmi-uid"} + err := ensureExpectedCRCVMIRunning(context.Background(), client.Resource(crcVMIGVR).Namespace(cfg.Namespace), cfg) + if !errors.Is(err, errCRCVMINoLongerCurrent) { + t.Fatalf("ensureExpectedCRCVMIRunning error = %v, want VMI lifecycle error", err) + } +} + +func TestMonitorCRCVMILifecycleCancelsWhenVMIStopsRunning(t *testing.T) { + client := newVMIDynamicClient(t, testVMIUID) + cfg := config{Namespace: testVMINamespace, InstanceName: testVMIName, ExpectedVMIUID: testVMIUID} + ctx, stop, err := monitorCRCVMILifecycle(context.Background(), client, cfg, discardLog{}) + if err != nil { + t.Fatalf("monitorCRCVMILifecycle: %v", err) + } + defer stop() + + vmis := client.Resource(crcVMIGVR).Namespace(cfg.Namespace) + vmi, err := vmis.Get(context.Background(), cfg.InstanceName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("getting VMI: %v", err) + } + if err := unstructured.SetNestedField(vmi.Object, "Failed", "status", "phase"); err != nil { + t.Fatalf("setting VMI phase: %v", err) + } + if _, err := vmis.Update(context.Background(), vmi, metav1.UpdateOptions{}); err != nil { + t.Fatalf("updating VMI: %v", err) + } + + select { + case <-ctx.Done(): + if !errors.Is(context.Cause(ctx), errCRCVMINoLongerCurrent) { + t.Fatalf("context cause = %v, want VMI lifecycle error", context.Cause(ctx)) + } + case <-time.After(5 * time.Second): + t.Fatal("non-running VMI did not cancel the agent context") + } +} + +func TestEnsureExpectedCRCVMIRunningRejectsNonRunningVMI(t *testing.T) { + client := newVMIDynamicClient(t, testVMIUID) + cfg := config{Namespace: testVMINamespace, InstanceName: testVMIName, ExpectedVMIUID: testVMIUID} + vmis := client.Resource(crcVMIGVR).Namespace(cfg.Namespace) + vmi, err := vmis.Get(context.Background(), cfg.InstanceName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("getting VMI: %v", err) + } + if err := unstructured.SetNestedField(vmi.Object, "Pending", "status", "phase"); err != nil { + t.Fatalf("setting VMI phase: %v", err) + } + if _, err := vmis.Update(context.Background(), vmi, metav1.UpdateOptions{}); err != nil { + t.Fatalf("updating VMI: %v", err) + } + + err = ensureExpectedCRCVMIRunning(context.Background(), vmis, cfg) + if !errors.Is(err, errCRCVMINoLongerCurrent) { + t.Fatalf("ensureExpectedCRCVMIRunning error = %v, want VMI lifecycle error", err) + } +} + +func TestWaitForCRCVMITerminationIgnoresBookmarkWithoutUID(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + w := watch.NewRaceFreeFake() + cfg := config{Namespace: testVMINamespace, InstanceName: testVMIName, ExpectedVMIUID: testVMIUID} + result := make(chan error, 1) + go func() { result <- waitForCRCVMITermination(ctx, w, cfg) }() + + w.Action(watch.Bookmark, &unstructured.Unstructured{Object: map[string]interface{}{ + "metadata": map[string]interface{}{"resourceVersion": "2"}, + }}) + + select { + case err := <-result: + t.Fatalf("watch ended after bookmark: %v", err) + case <-time.After(100 * time.Millisecond): + } + + cancel() + if err := <-result; err != nil { + t.Fatalf("watch error after cancellation: %v", err) + } +} + +func newVMIDynamicClient(t *testing.T, uid string) *fake.FakeDynamicClient { + t.Helper() + vmi := &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "kubevirt.io/v1", + "kind": "VirtualMachineInstance", + "metadata": map[string]interface{}{ + "name": testVMIName, + "namespace": testVMINamespace, + "uid": uid, + }, + "status": map[string]interface{}{"phase": "Running"}, + }} + vmi.SetUID(types.UID(uid)) + return fake.NewSimpleDynamicClient(runtime.NewScheme(), vmi) +} + +type discardLog struct{} + +func (discardLog) Info(string, ...any) {} diff --git a/internal/resources/crcidentity.go b/internal/resources/crcidentity.go new file mode 100644 index 0000000..9d47f73 --- /dev/null +++ b/internal/resources/crcidentity.go @@ -0,0 +1,229 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "bytes" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "time" +) + +const ( + crcIdentityValidity = 10 * 365 * 24 * time.Hour + crcIdentityKeySize = 2048 + pemCertificateType = "CERTIFICATE" + + CRCIdentityClientCAKey = "client-ca.crt" + CRCIdentityClientCertKey = "client.crt" + CRCIdentityClientPrivateKey = "client.key" + CRCIdentityServingCertKey = "serving.crt" + CRCIdentityServingPrivateKey = "serving.key" +) + +// CRCIdentity holds the long-lived credentials for a CRC ClusterInstance. +// The client CA private key is deliberately discarded after initial creation. +type CRCIdentity struct { + ClientCA []byte + ClientCert []byte + ClientPrivateKey []byte + ServingCert []byte + ServingPrivateKey []byte +} + +// CRCIdentitySecretName returns the Secret name for a CRC instance identity. +func CRCIdentitySecretName(instanceName string) string { + return instanceName + "-crc-identity" +} + +// SecretData returns identity material using the stable Secret data keys. +func (i CRCIdentity) SecretData() map[string][]byte { + return map[string][]byte{ + CRCIdentityClientCAKey: i.ClientCA, + CRCIdentityClientCertKey: i.ClientCert, + CRCIdentityClientPrivateKey: i.ClientPrivateKey, + CRCIdentityServingCertKey: i.ServingCert, + CRCIdentityServingPrivateKey: i.ServingPrivateKey, + } +} + +// CRCIdentityFromSecretData validates and returns Secret identity data. +func CRCIdentityFromSecretData(data map[string][]byte, hostname string) (CRCIdentity, error) { + identity := CRCIdentity{ + ClientCA: data[CRCIdentityClientCAKey], + ClientCert: data[CRCIdentityClientCertKey], + ClientPrivateKey: data[CRCIdentityClientPrivateKey], + ServingCert: data[CRCIdentityServingCertKey], + ServingPrivateKey: data[CRCIdentityServingPrivateKey], + } + if err := identity.Validate(hostname); err != nil { + return CRCIdentity{}, err + } + return identity, nil +} + +// NewCRCIdentity creates the credentials that remain stable for the complete +// ClusterInstance lifetime. +func NewCRCIdentity(hostname string) (CRCIdentity, error) { + if hostname == "" { + return CRCIdentity{}, fmt.Errorf("hostname must not be empty") + } + now := time.Now() + caKey, err := rsa.GenerateKey(rand.Reader, crcIdentityKeySize) + if err != nil { + return CRCIdentity{}, fmt.Errorf("generating client CA key: %w", err) + } + caTemplate := &x509.Certificate{ + SerialNumber: randomSerial(), + Subject: pkix.Name{CommonName: "admin-kubeconfig-signer-custom", OrganizationalUnit: []string{"openshift"}}, + NotBefore: now, NotAfter: now.Add(crcIdentityValidity), IsCA: true, BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + } + caDER, err := x509.CreateCertificate(rand.Reader, caTemplate, caTemplate, &caKey.PublicKey, caKey) + if err != nil { + return CRCIdentity{}, fmt.Errorf("creating client CA certificate: %w", err) + } + caCert, err := x509.ParseCertificate(caDER) + if err != nil { + return CRCIdentity{}, fmt.Errorf("parsing client CA certificate: %w", err) + } + clientKey, err := rsa.GenerateKey(rand.Reader, crcIdentityKeySize) + if err != nil { + return CRCIdentity{}, fmt.Errorf("generating client key: %w", err) + } + clientTemplate := &x509.Certificate{ + SerialNumber: randomSerial(), + Subject: pkix.Name{CommonName: "system:admin", OrganizationalUnit: []string{"system:masters"}}, + NotBefore: now, NotAfter: now.Add(crcIdentityValidity), BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + } + clientDER, err := x509.CreateCertificate(rand.Reader, clientTemplate, caCert, &clientKey.PublicKey, caKey) + if err != nil { + return CRCIdentity{}, fmt.Errorf("creating client certificate: %w", err) + } + servingKey, err := rsa.GenerateKey(rand.Reader, crcIdentityKeySize) + if err != nil { + return CRCIdentity{}, fmt.Errorf("generating serving key: %w", err) + } + servingTemplate := &x509.Certificate{ + SerialNumber: randomSerial(), Subject: pkix.Name{CommonName: hostname}, DNSNames: []string{hostname}, + NotBefore: now, NotAfter: now.Add(crcIdentityValidity), IsCA: true, BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + } + servingDER, err := x509.CreateCertificate(rand.Reader, servingTemplate, servingTemplate, &servingKey.PublicKey, servingKey) + if err != nil { + return CRCIdentity{}, fmt.Errorf("creating serving certificate: %w", err) + } + identity := CRCIdentity{ + ClientCA: pem.EncodeToMemory(&pem.Block{Type: pemCertificateType, Bytes: caDER}), + ClientCert: pem.EncodeToMemory(&pem.Block{Type: pemCertificateType, Bytes: clientDER}), + ClientPrivateKey: pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(clientKey)}), + ServingCert: pem.EncodeToMemory(&pem.Block{Type: pemCertificateType, Bytes: servingDER}), + ServingPrivateKey: pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(servingKey)}), + } + return identity, identity.Validate(hostname) +} + +// Validate rejects incomplete or unexpected identity material. Reconciliation +// must fail rather than silently replacing a corrupted stable identity. +func (i CRCIdentity) Validate(hostname string) error { + if hostname == "" { + return fmt.Errorf("hostname must not be empty") + } + ca, err := parseCertificate(i.ClientCA) + if err != nil || !ca.IsCA || !certificateCurrent(ca) { + return fmt.Errorf("invalid client CA") + } + client, err := parseCertificate(i.ClientCert) + if err != nil || !certificateCurrent(client) || client.Subject.CommonName != "system:admin" || !hasOrganizationalUnit(client, "system:masters") || !hasUsage(client, x509.ExtKeyUsageClientAuth) || client.CheckSignatureFrom(ca) != nil { + return fmt.Errorf("invalid client certificate") + } + if !keyMatchesCertificate(i.ClientPrivateKey, client) { + return fmt.Errorf("client private key does not match certificate") + } + serving, err := parseCertificate(i.ServingCert) + if err != nil || !certificateCurrent(serving) || serving.Subject.CommonName != hostname || len(serving.DNSNames) != 1 || serving.DNSNames[0] != hostname || serving.VerifyHostname(hostname) != nil || !hasUsage(serving, x509.ExtKeyUsageServerAuth) || serving.CheckSignatureFrom(serving) != nil { + return fmt.Errorf("invalid serving certificate") + } + if !keyMatchesCertificate(i.ServingPrivateKey, serving) { + return fmt.Errorf("serving private key does not match certificate") + } + return nil +} + +func certificateCurrent(cert *x509.Certificate) bool { + now := time.Now() + return !now.Before(cert.NotBefore) && now.Before(cert.NotAfter) +} + +func hasOrganizationalUnit(cert *x509.Certificate, want string) bool { + for _, unit := range cert.Subject.OrganizationalUnit { + if unit == want { + return true + } + } + return false +} + +func parseCertificate(certPEM []byte) (*x509.Certificate, error) { + block, _ := pem.Decode(certPEM) + if block == nil || block.Type != "CERTIFICATE" { + return nil, fmt.Errorf("no certificate PEM block") + } + return x509.ParseCertificate(block.Bytes) +} + +func keyMatchesCertificate(keyPEM []byte, cert *x509.Certificate) bool { + block, _ := pem.Decode(keyPEM) + if block == nil { + return false + } + key, err := x509.ParsePKCS1PrivateKey(block.Bytes) + if err != nil { + return false + } + certPublic, err := x509.MarshalPKIXPublicKey(cert.PublicKey) + if err != nil { + return false + } + keyPublic, err := x509.MarshalPKIXPublicKey(&key.PublicKey) + return err == nil && bytes.Equal(certPublic, keyPublic) +} + +func hasUsage(cert *x509.Certificate, want x509.ExtKeyUsage) bool { + for _, usage := range cert.ExtKeyUsage { + if usage == want { + return true + } + } + return false +} + +func randomSerial() *big.Int { + serial, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128)) + if err != nil { + return big.NewInt(1) + } + return serial +} diff --git a/internal/resources/crcidentity_test.go b/internal/resources/crcidentity_test.go new file mode 100644 index 0000000..2ea7119 --- /dev/null +++ b/internal/resources/crcidentity_test.go @@ -0,0 +1,48 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import "testing" + +func TestCRCIdentity(t *testing.T) { + hostname := "api.cluster.example.test" + identity, err := NewCRCIdentity(hostname) + if err != nil { + t.Fatalf("NewCRCIdentity: %v", err) + } + if err := identity.Validate(hostname); err != nil { + t.Fatalf("Validate: %v", err) + } + if _, err := CRCIdentityFromSecretData(identity.SecretData(), hostname); err != nil { + t.Fatalf("CRCIdentityFromSecretData: %v", err) + } + if err := identity.Validate("api.other.example.test"); err == nil { + t.Fatal("Validate accepted a different hostname") + } +} + +func TestCRCIdentityRejectsCorruptData(t *testing.T) { + identity, err := NewCRCIdentity("api.cluster.example.test") + if err != nil { + t.Fatalf("NewCRCIdentity: %v", err) + } + data := identity.SecretData() + data[CRCIdentityClientPrivateKey] = []byte("not a private key") + if _, err := CRCIdentityFromSecretData(data, "api.cluster.example.test"); err == nil { + t.Fatal("CRCIdentityFromSecretData accepted corrupt data") + } +} diff --git a/internal/resources/kascert.go b/internal/resources/kascert.go index 8abc990..cc1f41d 100644 --- a/internal/resources/kascert.go +++ b/internal/resources/kascert.go @@ -35,7 +35,9 @@ import ( // briefly invalidates any cached copy an external client holds (see // ServingCertNeedsRegen's doc comment). The validity window is therefore // long-lived. -const kasServingCertValidity = 10 * 365 * 24 * time.Hour +const ( + kasServingCertValidity = 10 * 365 * 24 * time.Hour +) // kasServingCertRenewBefore is how far ahead of actual expiry // ServingCertNeedsRegen requests regeneration. This gives a reconcile @@ -107,7 +109,7 @@ func GenerateAPIServerServingCert(hostname string) (certPEM, keyPEM []byte, err return nil, nil, fmt.Errorf("marshaling private key: %w", err) } - certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + certPEM = pem.EncodeToMemory(&pem.Block{Type: pemCertificateType, Bytes: der}) keyPEM = pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) return certPEM, keyPEM, nil } From 0f6e6ccd54b12fb2e54b7105d6468829f2bba390 Mon Sep 17 00:00:00 2001 From: Caleb Xu Date: Fri, 4 Sep 2026 12:48:29 -0400 Subject: [PATCH 3/5] feat(crc): fence agent handoffs to VMI identity Signed-off-by: Caleb Xu Assisted-by: OpenCode (GPT-5.6 Terra) --- README.md | 19 +- api/v1alpha1/clusterinstance_types.go | 3 + ...uestcluster.opdev.io_clusterinstances.yaml | 5 + config/rbac/crc_agent_role.yaml | 12 +- docs/reference/crd-api.md | 1 + .../controller/clusterinstance_controller.go | 55 +-- internal/controller/clusterinstance_crc.go | 358 ++++++++++++++++-- ...lusterinstance_leaseref_projection_test.go | 2 +- internal/resources/common.go | 40 +- internal/resources/crcagent.go | 15 +- internal/resources/crcagent_test.go | 38 ++ 11 files changed, 459 insertions(+), 89 deletions(-) create mode 100644 internal/resources/crcagent_test.go diff --git a/README.md b/README.md index 456d0e7..bfc4414 100644 --- a/README.md +++ b/README.md @@ -113,13 +113,14 @@ Two controllers implement this: below). The controller also creates a KubeVirt `VirtualMachine` wired to that volume. Once the VM's `VirtualMachineInstance` reports an IP, the controller creates a run-to-completion **crc-agent Job** - (`-crc-agent`). This Job connects to the VM over SSH as user + (`-crc-agent-`). This Job connects to the VM over SSH as user `core`, using `template.bundleSSHKeyRef`. It runs every post-boot fixup natively, with no external orchestration binary (see [crc-agent](#crc-agent-cmdcrc-agent) below). The controller waits for the Job to publish the raw kubeconfig Secret - (`-crc-raw-kubeconfig`). Only then does it mark the instance - `Ready`. KubeVirt is the only hypervisor involved. No nested + (`-crc-raw-kubeconfig-`). It verifies the external API + before it marks the instance `Ready`, and retains the result for recovery. + KubeVirt is the only hypervisor involved. No nested virtualization is required. - `hcp`: The controller creates a HyperShift `HostedCluster` with `platform: KubeVirt`. It sets `controllerAvailabilityPolicy` from @@ -414,7 +415,7 @@ For each `ClusterInstance` of topology `crc`, once its `VirtualMachineInstance` reports an IP, `ClusterInstanceReconciler` ensures a `Service` and passthrough `Route` that expose the guest API externally (see below). It then creates a run-to-completion **Kubernetes -Job** (`-crc-agent`, see `internal/resources.BuildCRCAgentJob`) +Job** (`-crc-agent-`, see `internal/resources.BuildCRCAgentJob`) that runs this binary. The binary: 1. Waits for the VM's SSH endpoint (port 22) to accept connections. It @@ -447,14 +448,16 @@ that runs this binary. The binary: `https://:443`. It embeds the same self-signed certificate as the trusted CA. It derives the cluster's OpenShift version from the typed `ClusterVersion` object. -6. Publishes both values into `-crc-raw-kubeconfig`, with keys - `kubeconfig` and `ocpVersion`. This Secret forms the **only** contract +6. Publishes both values and the configured VMI UID into + `-crc-raw-kubeconfig-`, with keys `kubeconfig`, + `ocpVersion`, and `vmiUID`. This Secret forms the **only** contract between crc-agent and `ClusterInstanceReconciler`. `ClusterInstanceReconciler` reads this Secret and never connects over SSH itself. `BuildCRCAgentJob` sets configuration through environment variables: `INSTANCE_NAME`, `INSTANCE_NAMESPACE`, `CRC_SSH_HOST` (the VM's IP), +`CRC_VMI_UID` (the VMI identity), `CRC_API_HOSTNAME` (the externally routable Route host), and `CRC_SSH_KEY_PATH` and `PULL_SECRET_PATH` (mounted Secret file paths). @@ -524,8 +527,8 @@ variable, the other a runtime environment variable, read through `os.Getenv` by both `ClusterInstanceReconciler` and `CRCBundleReconciler`. For the crc-agent Job, the image must run as the `crc-agent` -`ServiceAccount` (`config/rbac/crc_agent_*.yaml`), scoped to `secrets` -access in the operator's namespace only. For the bundle-prep Job, it must +`ServiceAccount` (`config/rbac/crc_agent_*.yaml`), scoped to `secrets` and +its `VirtualMachineInstance` in the operator's namespace. For the bundle-prep Job, it must run as the `bundle-prep` `ServiceAccount`, scoped to `secrets` and `configmaps` access in the operator's namespace only. diff --git a/api/v1alpha1/clusterinstance_types.go b/api/v1alpha1/clusterinstance_types.go index 16ca415..bdf5494 100644 --- a/api/v1alpha1/clusterinstance_types.go +++ b/api/v1alpha1/clusterinstance_types.go @@ -77,6 +77,9 @@ type CRCBackingStatus struct { // SSHEndpoint is host:port used by the crc-agent to reach the CRC VM for // post-boot fixups and kubeconfig extraction. SSHEndpoint string `json:"sshEndpoint,omitempty"` + // VMIUID identifies the VirtualMachineInstance for which the crc-agent + // completed its post-boot handoff. + VMIUID string `json:"vmiUID,omitempty"` } // HyperShiftBackingStatus tracks the HostedCluster/NodePool backing a topology=hcp diff --git a/config/crd/bases/guestcluster.opdev.io_clusterinstances.yaml b/config/crd/bases/guestcluster.opdev.io_clusterinstances.yaml index 5f87742..8e411f5 100644 --- a/config/crd/bases/guestcluster.opdev.io_clusterinstances.yaml +++ b/config/crd/bases/guestcluster.opdev.io_clusterinstances.yaml @@ -357,6 +357,11 @@ spec: description: VMName is the name of the KubeVirt VirtualMachine running the CRC/SNO bundle. type: string + vmiUID: + description: |- + VMIUID identifies the VirtualMachineInstance for which the crc-agent + completed its post-boot handoff. + type: string type: object hyperShift: description: HyperShift holds backing-object references for topology=hcp diff --git a/config/rbac/crc_agent_role.yaml b/config/rbac/crc_agent_role.yaml index af9ec51..85d9a0c 100644 --- a/config/rbac/crc_agent_role.yaml +++ b/config/rbac/crc_agent_role.yaml @@ -8,14 +8,18 @@ metadata: namespace: system rules: - apiGroups: - - "" + - kubevirt.io resources: - - secrets + - virtualmachineinstances verbs: - get - list - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - get - create - update - - patch - - delete diff --git a/docs/reference/crd-api.md b/docs/reference/crd-api.md index 70c6ee2..a6f0c2b 100644 --- a/docs/reference/crd-api.md +++ b/docs/reference/crd-api.md @@ -36,6 +36,7 @@ _Appears in:_ | `vmName` _string_ | VMName is the name of the KubeVirt VirtualMachine running the CRC/SNO bundle. | | | | `dataVolumeName` _string_ | DataVolumeName is the CDI DataVolume providing the VM's root disk. | | | | `sshEndpoint` _string_ | SSHEndpoint is host:port used by the crc-agent to reach the CRC VM for
post-boot fixups and kubeconfig extraction. | | | +| `vmiUID` _string_ | VMIUID identifies the VirtualMachineInstance for which the crc-agent
completed its post-boot handoff. | | | #### CRCBundle diff --git a/internal/controller/clusterinstance_controller.go b/internal/controller/clusterinstance_controller.go index 3a45bd8..51fbabc 100644 --- a/internal/controller/clusterinstance_controller.go +++ b/internal/controller/clusterinstance_controller.go @@ -35,6 +35,8 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" + kubevirtv1 "kubevirt.io/api/core/v1" + brokerv1alpha1 "github.com/caxu-rh/guestcluster-operator/api/v1alpha1" "github.com/caxu-rh/guestcluster-operator/internal/resources" ) @@ -48,8 +50,9 @@ const ( // (VM boot, HostedCluster provisioning) to progress. requeueInterval = 20 * time.Second - conditionTypeReady = "Ready" - conditionTypeVersionMismatch = "VersionMismatch" + conditionTypeReady = "Ready" + conditionTypeVersionMismatch = "VersionMismatch" + conditionTypeGuestAPIReachable = "GuestAPIReachable" ) // ClusterInstanceReconciler reconciles a ClusterInstance object @@ -139,14 +142,10 @@ func (r *ClusterInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } - // Ready instances are steady state for backing-resource provisioning. - // Re-running reconcileCRC/reconcileHyperShift here would be harmless, - // because both are Get-or-Create idempotent, but wasteful, because this - // reconciler is now also triggered by unrelated ClusterLease changes - // (see SetupWithManager) only to keep the derived LeaseRef projection - // current. The only thing a Ready instance still needs from this - // reconciler is that projection update. if instance.Status.Phase == brokerv1alpha1.PhaseReady { + if instance.Spec.Type == brokerv1alpha1.TopologyCRC { + return r.reconcileReadyCRC(ctx, instance) + } return r.reconcileLeaseRefProjection(ctx, instance) } @@ -201,6 +200,13 @@ func (r *ClusterInstanceReconciler) reconcileLeaseRefProjection(ctx context.Cont } func (r *ClusterInstanceReconciler) reconcileCRC(ctx context.Context, instance *brokerv1alpha1.ClusterInstance) (ctrl.Result, error) { + if result, err := r.reconcileProvisioningCRCVMI(ctx, instance); result != nil || err != nil { + if result == nil { + return ctrl.Result{}, err + } + return *result, err + } + pullSecretName, err := r.resolvePullSecret(ctx, instance, instance.Namespace) if err != nil { return r.markFailedWithReason(ctx, instance, "InvalidPullSecret", err) @@ -228,6 +234,7 @@ func (r *ClusterInstanceReconciler) reconcileCRC(ctx context.Context, instance * VMName: res.vmName, DataVolumeName: res.dvName, SSHEndpoint: res.sshEndpoint, + VMIUID: res.vmiUID, } if !res.ready { @@ -357,22 +364,6 @@ func (r *ClusterInstanceReconciler) markReady(ctx context.Context, instance *bro return ctrl.Result{}, fmt.Errorf("updating status to Ready: %w", err) } - // markReady has now durably copied the raw crc-agent handoff Secret - // (topology=crc only) into the canonical secret above. Delete the raw - // secret so it does not linger as a permanent duplicate. This deletion - // happens only after the canonical secret and the status update both - // succeed, so a failure earlier in this function, if requeued and - // retried, never loses the only copy of the kubeconfig. - if instance.Spec.Type == brokerv1alpha1.TopologyCRC { - raw := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ - Name: resources.RawKubeconfigSecretName(instance.Name), - Namespace: instance.Namespace, - }} - if err := r.deleteIfExists(ctx, raw, "consumed raw kubeconfig secret"); err != nil { - return ctrl.Result{}, err - } - } - return ctrl.Result{}, nil } @@ -578,11 +569,25 @@ func (r *ClusterInstanceReconciler) instanceForLease(_ context.Context, obj clie }} } +// instanceForVMI maps the deterministically named CRC VMI to its +// ClusterInstance. This lets the controller recover as soon as KubeVirt +// replaces a VMI, instead of waiting for an unrelated instance update. +func (r *ClusterInstanceReconciler) instanceForVMI(_ context.Context, obj client.Object) []reconcile.Request { + vmi, ok := obj.(*kubevirtv1.VirtualMachineInstance) + if !ok { + return nil + } + return []reconcile.Request{{ + NamespacedName: client.ObjectKey{Namespace: vmi.Namespace, Name: vmi.Name}, + }} +} + // SetupWithManager sets up the controller with the Manager. func (r *ClusterInstanceReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&brokerv1alpha1.ClusterInstance{}). Watches(&brokerv1alpha1.ClusterLease{}, handler.EnqueueRequestsFromMapFunc(r.instanceForLease)). + Watches(&kubevirtv1.VirtualMachineInstance{}, handler.EnqueueRequestsFromMapFunc(r.instanceForVMI)). Named("clusterinstance"). Complete(r) } diff --git a/internal/controller/clusterinstance_crc.go b/internal/controller/clusterinstance_crc.go index 756b929..4470fbb 100644 --- a/internal/controller/clusterinstance_crc.go +++ b/internal/controller/clusterinstance_crc.go @@ -19,14 +19,22 @@ package controller import ( "context" "fmt" + "net/http" "os" + "strings" + "time" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" kubevirtv1 "kubevirt.io/api/core/v1" @@ -39,6 +47,8 @@ import ( "github.com/caxu-rh/guestcluster-operator/internal/resources" ) +const crcReadyRequeueInterval = time.Minute + // crcResult carries the outcome of reconciling a topology=crc instance's // backing objects. The caller (Reconcile) uses this outcome to decide phase // transitions, so this function does not need to know about phase @@ -50,6 +60,7 @@ type crcResult struct { vmName string dvName string sshEndpoint string + vmiUID string // apiEndpoint is the externally-routable URL of the guest API server // (the passthrough Route host, see ensureCRCAPIRoute). markReady copies // it into ClusterInstanceStatus.APIEndpoint / ClusterLeaseStatus.APIEndpoint. @@ -155,7 +166,6 @@ func (r *ClusterInstanceReconciler) ensureCRCBacking(ctx context.Context, instan } else if err != nil { return res, fmt.Errorf("getting CRC DataVolume %s/%s: %w", dv.Namespace, dv.Name, err) } - vm := resources.BuildCRCVirtualMachine(instance, res.dvName) existingVM := &kubevirtv1.VirtualMachine{} if err := r.Get(ctx, types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, existingVM); apierrors.IsNotFound(err) { @@ -183,6 +193,11 @@ func (r *ClusterInstanceReconciler) ensureCRCBacking(ctx context.Context, instan } else if err != nil { return res, fmt.Errorf("getting CRC VirtualMachineInstance %s/%s: %w", vm.Namespace, vm.Name, err) } + if vmi.Status.Phase != kubevirtv1.Running { + log.Info("CRC VMI is not running yet, waiting", "virtualMachine", vm.Name, "phase", vmi.Status.Phase) + return res, nil + } + res.vmiUID = string(vmi.UID) var vmIP string for _, iface := range vmi.Status.Interfaces { @@ -208,12 +223,19 @@ func (r *ClusterInstanceReconciler) ensureCRCBacking(ctx context.Context, instan return res, err } res.apiEndpoint = "https://" + apiHost + identitySecretName, err := r.ensureCRCIdentity(ctx, instance, apiHost) + if err != nil { + return res, err + } // Ensure the crc-agent Job exists. It SSHes into the VM and runs the // post-boot fixups natively (see cmd/crc-agent). This call is // idempotent: once created, the Job runs to completion, or exhausts its // BackoffLimit, on its own. - job := resources.BuildCRCAgentJob(instance, vmIP, sshSecretName, sshDataKey, crcAgentImage(), apiHost, pullSecretName) + job := resources.BuildCRCAgentJob(instance, vmIP, res.vmiUID, sshSecretName, sshDataKey, identitySecretName, crcAgentImage(), apiHost, pullSecretName) + if err := controllerutil.SetControllerReference(instance, job, r.Scheme); err != nil { + return res, fmt.Errorf("setting owner reference on crc-agent Job %s/%s: %w", job.Namespace, job.Name, err) + } if err := r.Get(ctx, types.NamespacedName{Name: job.Name, Namespace: job.Namespace}, &batchv1.Job{}); apierrors.IsNotFound(err) { if err := r.Create(ctx, job); err != nil && !apierrors.IsAlreadyExists(err) { return res, fmt.Errorf("creating crc-agent Job %s/%s: %w", job.Namespace, job.Name, err) @@ -233,6 +255,10 @@ func (r *ClusterInstanceReconciler) ensureCRCBacking(ctx context.Context, instan if len(kubeconfig) == 0 { return res, nil // not published yet; checkCRCKubeconfigHandoff already logged why } + if err := checkCRCAPIReady(ctx, kubeconfig); err != nil { + log.Info("CRC guest API is not externally ready yet", "error", err) + return res, nil + } res.ready = true res.kubeconfig = kubeconfig @@ -240,31 +266,272 @@ func (r *ClusterInstanceReconciler) ensureCRCBacking(ctx context.Context, instan return res, nil } +// ensureCRCIdentity creates the credentials that stay stable while this +// ClusterInstance exists. VMI recovery intentionally does not delete it. +func (r *ClusterInstanceReconciler) ensureCRCIdentity(ctx context.Context, instance *brokerv1alpha1.ClusterInstance, apiHostname string) (string, error) { + name := resources.CRCIdentitySecretName(instance.Name) + existing := &corev1.Secret{} + key := types.NamespacedName{Name: name, Namespace: instance.Namespace} + if err := r.Get(ctx, key, existing); err == nil { + if _, err := resources.CRCIdentityFromSecretData(existing.Data, apiHostname); err != nil { + return "", fmt.Errorf("validating CRC identity secret %s/%s: %w", key.Namespace, key.Name, err) + } + if !metav1.IsControlledBy(existing, instance) { + return "", fmt.Errorf("CRC identity secret %s/%s is not controlled by this ClusterInstance", key.Namespace, key.Name) + } + return name, nil + } else if !apierrors.IsNotFound(err) { + return "", fmt.Errorf("getting CRC identity secret %s/%s: %w", key.Namespace, key.Name, err) + } + + identity, err := resources.NewCRCIdentity(apiHostname) + if err != nil { + return "", fmt.Errorf("generating CRC identity: %w", err) + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: instance.Namespace, Labels: resources.CommonLabels(instance)}, + Type: corev1.SecretTypeOpaque, + Data: identity.SecretData(), + } + if err := controllerutil.SetControllerReference(instance, secret, r.Scheme); err != nil { + return "", fmt.Errorf("setting owner reference on CRC identity secret %s/%s: %w", key.Namespace, key.Name, err) + } + if err := r.Create(ctx, secret); err != nil { + if !apierrors.IsAlreadyExists(err) { + return "", fmt.Errorf("creating CRC identity secret %s/%s: %w", key.Namespace, key.Name, err) + } + if err := r.Get(ctx, key, existing); err != nil { + return "", fmt.Errorf("getting concurrently created CRC identity secret %s/%s: %w", key.Namespace, key.Name, err) + } + if _, err := resources.CRCIdentityFromSecretData(existing.Data, apiHostname); err != nil || !metav1.IsControlledBy(existing, instance) { + return "", fmt.Errorf("validating concurrently created CRC identity secret %s/%s", key.Namespace, key.Name) + } + } + return name, nil +} + +// reconcileReadyCRC verifies that the published kubeconfig still belongs to +// the running VMI and can reach the guest API before preserving Ready. +func (r *ClusterInstanceReconciler) reconcileReadyCRC(ctx context.Context, instance *brokerv1alpha1.ClusterInstance) (ctrl.Result, error) { + vmi := &kubevirtv1.VirtualMachineInstance{} + key := types.NamespacedName{Name: resources.VMName(instance.Name), Namespace: instance.Namespace} + if err := r.Get(ctx, key, vmi); err != nil { + if apierrors.IsNotFound(err) { + return r.invalidateCRCReadiness(ctx, instance, "", "VMIUnavailable", "CRC VirtualMachineInstance is not running") + } + return ctrl.Result{}, fmt.Errorf("getting CRC VirtualMachineInstance %s/%s: %w", key.Namespace, key.Name, err) + } + if vmi.Status.Phase != kubevirtv1.Running { + return r.invalidateCRCReadiness(ctx, instance, string(vmi.UID), "VMIUnavailable", "CRC VirtualMachineInstance is not running") + } + + previousUID := "" + if instance.Status.CRC != nil { + previousUID = instance.Status.CRC.VMIUID + } + currentUID := string(vmi.UID) + if crcVMIChanged(previousUID, currentUID) { + reason := "VMIReplaced" + message := "CRC VirtualMachineInstance was replaced; rerunning post-boot setup" + if previousUID == "" || currentUID == "" { + reason = "VMIIdentityUnknown" + message = "CRC VirtualMachineInstance identity was not recorded; rerunning post-boot setup" + } + return r.invalidateCRCReadiness(ctx, instance, currentUID, reason, message) + } + + published := &corev1.Secret{} + publishedName := resources.KubeconfigSecretName(instance.Name) + if err := r.Get(ctx, types.NamespacedName{Name: publishedName, Namespace: instance.Namespace}, published); err != nil { + if apierrors.IsNotFound(err) { + kubeconfig, ocpVersion, handoffErr := r.checkCRCKubeconfigHandoff(ctx, instance) + if handoffErr != nil { + return ctrl.Result{}, handoffErr + } + if len(kubeconfig) == 0 { + return r.markCRCAPIUnavailable(ctx, instance, "KubeconfigUnavailable", "published CRC kubeconfig is missing") + } + if err := checkCRCAPIReady(ctx, kubeconfig); err != nil { + return r.markCRCAPIUnavailable(ctx, instance, "GuestAPIUnavailable", fmt.Sprintf("guest API readiness check failed: %v", err)) + } + return r.markReady(ctx, instance, ocpVersion, instance.Status.APIEndpoint, kubeconfig) + } + return ctrl.Result{}, fmt.Errorf("getting published kubeconfig secret %s/%s: %w", instance.Namespace, publishedName, err) + } + if err := checkCRCAPIReady(ctx, published.Data[resources.KubeconfigSecretKey]); err != nil { + return r.markCRCAPIUnavailable(ctx, instance, "GuestAPIUnavailable", fmt.Sprintf("guest API readiness check failed: %v", err)) + } + if result, err := r.recordCRCAPIHealth(ctx, instance, metav1.ConditionTrue, "GuestAPIReady", "guest API readiness check succeeded"); err != nil || result.RequeueAfter > 0 { + return result, err + } + + result, err := r.reconcileLeaseRefProjection(ctx, instance) + if err != nil || result.RequeueAfter > 0 { + return result, err + } + return ctrl.Result{RequeueAfter: crcReadyRequeueInterval}, nil +} + +// reconcileProvisioningCRCVMI records the VMI identity before the crc-agent +// handoff can be reused. A changed identity invalidates that handoff first. +func (r *ClusterInstanceReconciler) reconcileProvisioningCRCVMI(ctx context.Context, instance *brokerv1alpha1.ClusterInstance) (*ctrl.Result, error) { + vmi := &kubevirtv1.VirtualMachineInstance{} + key := types.NamespacedName{Name: resources.VMName(instance.Name), Namespace: instance.Namespace} + if err := r.Get(ctx, key, vmi); err != nil { + if apierrors.IsNotFound(err) { + return nil, nil + } + return nil, fmt.Errorf("getting CRC VirtualMachineInstance %s/%s: %w", key.Namespace, key.Name, err) + } + + previousUID := "" + if instance.Status.CRC != nil { + previousUID = instance.Status.CRC.VMIUID + } + // An initial provisioning reconcile can observe status before + // ensureCRCBacking records the VMI UID. That is not a VMI replacement. + if previousUID == "" { + return nil, nil + } + currentUID := string(vmi.UID) + if !crcVMIChanged(previousUID, currentUID) { + return nil, nil + } + + reason := "VMIReplaced" + message := "CRC VirtualMachineInstance was replaced; rerunning post-boot setup" + if previousUID == "" || currentUID == "" { + reason = "VMIIdentityUnknown" + message = "CRC VirtualMachineInstance identity was not recorded; rerunning post-boot setup" + } + result, err := r.invalidateCRCReadiness(ctx, instance, currentUID, reason, message) + return &result, err +} + +func crcVMIChanged(previousUID, currentUID string) bool { + return previousUID == "" || currentUID == "" || previousUID != currentUID +} + +func checkCRCAPIReady(ctx context.Context, kubeconfig []byte) error { + if len(kubeconfig) == 0 { + return fmt.Errorf("kubeconfig is empty") + } + config, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig) + if err != nil { + return fmt.Errorf("parsing kubeconfig: %w", err) + } + transport, err := rest.TransportFor(config) + if err != nil { + return fmt.Errorf("creating API transport: %w", err) + } + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + request, err := http.NewRequestWithContext(ctx, http.MethodGet, strings.TrimRight(config.Host, "/")+"/readyz", nil) + if err != nil { + return fmt.Errorf("creating readiness request: %w", err) + } + response, err := (&http.Client{Transport: transport}).Do(request) + if err != nil { + return fmt.Errorf("requesting guest API readiness: %w", err) + } + defer func() { _ = response.Body.Close() }() + if response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusMultipleChoices { + return fmt.Errorf("guest API returned %s", response.Status) + } + return nil +} + +// invalidateCRCReadiness removes the one-shot handoff from a previous VMI so +// the next reconcile creates a fresh crc-agent Job for the current VMI. +func (r *ClusterInstanceReconciler) invalidateCRCReadiness(ctx context.Context, instance *brokerv1alpha1.ClusterInstance, vmiUID, reason, message string) (ctrl.Result, error) { + if instance.Status.CRC != nil && instance.Status.CRC.VMIUID != "" { + job := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: resources.CRCAgentJobName(instance.Name, instance.Status.CRC.VMIUID), Namespace: instance.Namespace}} + if err := r.deleteIfExists(ctx, job, "stale crc-agent Job"); err != nil { + return ctrl.Result{}, err + } + } + published := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: resources.KubeconfigSecretName(instance.Name), Namespace: instance.Namespace}} + if err := r.deleteIfExists(ctx, published, "published CRC kubeconfig secret"); err != nil { + return ctrl.Result{}, err + } + + if instance.Status.CRC == nil { + instance.Status.CRC = &brokerv1alpha1.CRCBackingStatus{VMName: resources.VMName(instance.Name), DataVolumeName: resources.DataVolumeName(instance.Name)} + } + if vmiUID != "" { + instance.Status.CRC.VMIUID = vmiUID + } + instance.Status.Phase = brokerv1alpha1.PhaseProvisioning + instance.Status.APIEndpoint = "" + instance.Status.KubeconfigSecretRef = corev1.LocalObjectReference{} + apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: conditionTypeReady, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: instance.Generation, + }) + if err := r.Status().Update(ctx, instance); err != nil { + return ctrl.Result{}, fmt.Errorf("updating status while recovering CRC: %w", err) + } + return ctrl.Result{RequeueAfter: requeueInterval}, nil +} + +func (r *ClusterInstanceReconciler) recordCRCAPIHealth(ctx context.Context, instance *brokerv1alpha1.ClusterInstance, status metav1.ConditionStatus, reason, message string) (ctrl.Result, error) { + condition := apimeta.FindStatusCondition(instance.Status.Conditions, conditionTypeGuestAPIReachable) + if condition != nil && condition.Status == status && condition.Reason == reason && condition.Message == message { + return ctrl.Result{RequeueAfter: crcReadyRequeueInterval}, nil + } + apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: conditionTypeGuestAPIReachable, + Status: status, + Reason: reason, + Message: message, + ObservedGeneration: instance.Generation, + }) + if err := r.Status().Update(ctx, instance); err != nil { + return ctrl.Result{}, fmt.Errorf("updating CRC guest API health: %w", err) + } + return ctrl.Result{RequeueAfter: crcReadyRequeueInterval}, nil +} + +// markCRCAPIUnavailable prevents leases from using an unreachable guest API +// while retaining the VMI handoff and completed agent Job for a later retry. +func (r *ClusterInstanceReconciler) markCRCAPIUnavailable(ctx context.Context, instance *brokerv1alpha1.ClusterInstance, reason, message string) (ctrl.Result, error) { + instance.Status.Phase = brokerv1alpha1.PhaseProvisioning + instance.Status.KubeconfigSecretRef = corev1.LocalObjectReference{} + apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: conditionTypeReady, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: instance.Generation, + }) + apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: conditionTypeGuestAPIReachable, + Status: metav1.ConditionFalse, + Reason: reason, + Message: message, + ObservedGeneration: instance.Generation, + }) + if err := r.Status().Update(ctx, instance); err != nil { + return ctrl.Result{}, fmt.Errorf("updating status while waiting for CRC guest API: %w", err) + } + return ctrl.Result{RequeueAfter: requeueInterval}, nil +} + // checkCRCKubeconfigHandoff reports whether the crc-agent Job has published -// the raw kubeconfig handoff Secret (see resources.RawKubeconfigSecretName). -// This raw secret is transient: markReady deletes it once markReady has -// durably copied its contents into the canonical -kubeconfig -// secret (see markReady). It is normal for the raw secret to be gone again -// on later reconciles of an already-Ready instance, so checkCRCKubeconfigHandoff -// falls back to the canonical published secret in that case. Without this -// fallback, a Ready instance reconciling again (for example, on a periodic -// resync) would regress to Provisioning, because the one-time raw secret it -// originally consumed no longer exists. A nil kubeconfig with a nil error -// means the caller should wait and retry on a later reconcile. +// the VMI-specific raw kubeconfig handoff Secret. The controller retains this +// result so it can restore the canonical Secret without rerunning the agent. +// A nil kubeconfig with a nil error means the caller should wait and retry. func (r *ClusterInstanceReconciler) checkCRCKubeconfigHandoff(ctx context.Context, instance *brokerv1alpha1.ClusterInstance) ([]byte, string, error) { log := logf.FromContext(ctx) raw := &corev1.Secret{} - rawName := resources.RawKubeconfigSecretName(instance.Name) + if instance.Status.CRC == nil || instance.Status.CRC.VMIUID == "" { + return nil, "", nil + } + rawName := resources.RawKubeconfigSecretNameForVMI(instance.Name, instance.Status.CRC.VMIUID) if err := r.Get(ctx, types.NamespacedName{Name: rawName, Namespace: instance.Namespace}, raw); apierrors.IsNotFound(err) { - published := &corev1.Secret{} - publishedName := resources.KubeconfigSecretName(instance.Name) - if getErr := r.Get(ctx, types.NamespacedName{Name: publishedName, Namespace: instance.Namespace}, published); getErr == nil { - if kubeconfig := published.Data[resources.KubeconfigSecretKey]; len(kubeconfig) > 0 { - return kubeconfig, string(published.Data[resources.OCPVersionSecretKey]), nil - } - } else if !apierrors.IsNotFound(getErr) { - return nil, "", fmt.Errorf("getting published kubeconfig secret %s/%s: %w", instance.Namespace, publishedName, getErr) - } log.Info("CRC VM ready, awaiting crc-agent kubeconfig handoff", "secret", rawName) return nil, "", nil } else if err != nil { @@ -276,6 +543,10 @@ func (r *ClusterInstanceReconciler) checkCRCKubeconfigHandoff(ctx context.Contex log.Info("raw kubeconfig secret present but missing kubeconfig key, awaiting crc-agent", "secret", rawName) return nil, "", nil } + if instance.Status.CRC == nil || raw.Data[resources.VMIUIDSecretKey] == nil || string(raw.Data[resources.VMIUIDSecretKey]) != instance.Status.CRC.VMIUID { + log.Info("ignoring raw kubeconfig handoff for a different CRC VMI", "secret", rawName) + return nil, "", nil + } return kubeconfig, string(raw.Data[resources.OCPVersionSecretKey]), nil } @@ -348,16 +619,17 @@ func (r *ClusterInstanceReconciler) mgmtIngressDomain(ctx context.Context) (stri // through the normal creation path. See clusterinstance_controller.go's // Reconcile doc comment for the rationale. func (r *ClusterInstanceReconciler) teardownCRCBacking(ctx context.Context, instance *brokerv1alpha1.ClusterInstance) error { - // Delete the crc-agent Job first. It references the VM's IP, and its - // Pods should not keep running, or be left orphaned, against a VM that - // is about to be torn down. - job := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{ - Name: resources.CRCAgentJobName(instance.Name), - Namespace: instance.Namespace, - }} + // Delete all crc-agent Jobs before the VM. VMI-scoped Job names mean more + // than one completed Job can exist after VMI replacement. + jobs := &batchv1.JobList{} + if err := r.List(ctx, jobs, client.InNamespace(instance.Namespace), client.MatchingLabels(resources.CommonLabels(instance))); err != nil { + return fmt.Errorf("listing crc-agent Jobs: %w", err) + } background := metav1.DeletePropagationBackground - if err := r.deleteIfExists(ctx, job, "crc-agent Job", client.PropagationPolicy(background)); err != nil { - return err + for i := range jobs.Items { + if err := r.deleteIfExists(ctx, &jobs.Items[i], "crc-agent Job", client.PropagationPolicy(background)); err != nil { + return err + } } vm := &kubevirtv1.VirtualMachine{ObjectMeta: metav1.ObjectMeta{ @@ -376,12 +648,24 @@ func (r *ClusterInstanceReconciler) teardownCRCBacking(ctx context.Context, inst return err } - raw := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ - Name: resources.RawKubeconfigSecretName(instance.Name), - Namespace: instance.Namespace, - }} - if err := r.deleteIfExists(ctx, raw, "stale raw kubeconfig secret"); err != nil { - return err + rawSecrets := &corev1.SecretList{} + if err := r.List(ctx, rawSecrets, client.InNamespace(instance.Namespace), client.MatchingLabels{ + resources.LabelManagedBy: "crc-agent", + resources.LabelInstance: instance.Name, + }); err != nil { + return fmt.Errorf("listing CRC handoff secrets: %w", err) + } + for i := range rawSecrets.Items { + if err := r.deleteIfExists(ctx, &rawSecrets.Items[i], "raw kubeconfig secret"); err != nil { + return err + } + } + // Remove the legacy result name from older controller versions. + for _, name := range []string{resources.RawKubeconfigSecretName(instance.Name)} { + raw := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: instance.Namespace}} + if err := r.deleteIfExists(ctx, raw, "legacy raw kubeconfig secret"); err != nil { + return err + } } route := &routev1.Route{ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/clusterinstance_leaseref_projection_test.go b/internal/controller/clusterinstance_leaseref_projection_test.go index d2284ad..a6a0122 100644 --- a/internal/controller/clusterinstance_leaseref_projection_test.go +++ b/internal/controller/clusterinstance_leaseref_projection_test.go @@ -78,7 +78,7 @@ func newProjectionTestInstance(name string, leaseRef *corev1.LocalObjectReferenc Finalizers: []string{instanceFinalizer}, }, Spec: brokerv1alpha1.ClusterInstanceSpec{ - Type: brokerv1alpha1.TopologyCRC, + Type: brokerv1alpha1.TopologyHCP, Template: brokerv1alpha1.ClusterTemplate{ OCPVersion: testOCPVersion, Memory: testMemory, diff --git a/internal/resources/common.go b/internal/resources/common.go index c664521..4285de3 100644 --- a/internal/resources/common.go +++ b/internal/resources/common.go @@ -22,6 +22,8 @@ limitations under the License. package resources import ( + "crypto/sha256" + "encoding/hex" "fmt" "os" @@ -128,12 +130,16 @@ const KubeconfigSecretKey = "kubeconfig" // OCPVersionSecretKey is the Secret data key where the crc-agent writes the // observed in-guest OpenShift version, alongside the raw kubeconfig, for // topology=crc instances (see RawKubeconfigSecretName). The -// management-cluster ClusterInstance controller has no direct network path -// to the CRC guest API. It relies on the crc-agent, which reaches the guest -// API over the hypervisor-local VM network, to report this value instead of -// querying it directly. +// management-cluster ClusterInstance controller uses the published API route +// only for a readiness check. It relies on the crc-agent, which reaches the +// guest API over the hypervisor-local VM network, to report this value. const OCPVersionSecretKey = "ocpVersion" +// VMIUIDSecretKey is the Secret data key where crc-agent records the UID of +// the VMI it configured. The controller uses it to reject a handoff from a +// VMI that was deleted or replaced while the agent was still running. +const VMIUIDSecretKey = "vmiUID" + // RawKubeconfigSecretName defines the contract between the crc-agent and // the ClusterInstance controller for topology=crc instances. The crc-agent // runs with network access to the nested CRC VM. It drives in-guest @@ -147,6 +153,12 @@ func RawKubeconfigSecretName(instanceName string) string { return instanceName + "-crc-raw-kubeconfig" } +// RawKubeconfigSecretNameForVMI fences a handoff to one VMI. A stale agent +// can only write its own Secret and cannot replace the current VMI handoff. +func RawKubeconfigSecretNameForVMI(instanceName, vmiUID string) string { + return RawKubeconfigSecretName(instanceName) + "-" + vmiUID +} + // LeaseKubeconfigSecretName is the name of the Secret where the // ClusterLease controller copies a bound ClusterInstance's kubeconfig, in // the ClusterLease's own namespace. CI consumers read this Secret instead @@ -157,15 +169,23 @@ func LeaseKubeconfigSecretName(leaseName string) string { return leaseName + "-kubeconfig" } -// CRCAgentJobName is the deterministic name of the per-instance Kubernetes -// Job that runs the crc-agent container for a topology=crc ClusterInstance. +// CRCAgentJobName is the deterministic name of the per-VMI Kubernetes Job +// that runs the crc-agent container for a topology=crc ClusterInstance. // This is a run-to-completion Job, not a long-running Deployment. The // crc-agent's work is a one-shot task per VM boot: SSH into the freshly // booted CRC VM, run the native post-boot fixups, and publish -// RawKubeconfigSecretName. The operator reruns this task fresh on every -// recycle. -func CRCAgentJobName(instanceName string) string { - return instanceName + "-crc-agent" +// RawKubeconfigSecretName. The VMI hash keeps the name within the Kubernetes +// DNS label limit when an instance name is at its maximum length. +func CRCAgentJobName(instanceName, vmiUID string) string { + const suffix = "-crc-agent-" + const hashLength = 12 + hash := sha256.Sum256([]byte(vmiUID)) + uidHash := hex.EncodeToString(hash[:])[:hashLength] + maxPrefixLength := 63 - len(suffix) - hashLength + if len(instanceName) > maxPrefixLength { + instanceName = instanceName[:maxPrefixLength] + } + return instanceName + suffix + uidHash } // CRCAPIServiceName is the deterministic name of the ClusterIP Service that diff --git a/internal/resources/crcagent.go b/internal/resources/crcagent.go index 4069b48..ef2826b 100644 --- a/internal/resources/crcagent.go +++ b/internal/resources/crcagent.go @@ -105,15 +105,15 @@ func CRCAgentPullSecretPath() string { // image is the crc-agent container image to run (see CRCAgentImageEnvVar). // apiHostname is the externally routable hostname for which the // ClusterInstance controller already provisioned a passthrough Route (see -// BuildCRCAPIRoute); the crc-agent uses it to mint the guest API server's -// external-facing serving certificate and to rewrite the published +// BuildCRCAPIRoute); the crc-agent uses it to select the mounted guest API +// server certificate and to rewrite the published // kubeconfig's server URL. pullSecretName is the name (in // instance.Namespace) of the Secret holding the pull-secret to inject into // the guest cluster. The caller resolves it, either from the template's // explicit PullSecretRef, or from a materialized copy of the management // cluster's own default pull secret (see // ClusterInstanceReconciler.resolvePullSecret). -func BuildCRCAgentJob(instance *brokerv1alpha1.ClusterInstance, vmIP, sshKeySecretName, bundleKeyDataKey, image, apiHostname, pullSecretName string) *batchv1.Job { +func BuildCRCAgentJob(instance *brokerv1alpha1.ClusterInstance, vmIP, vmiUID, sshKeySecretName, bundleKeyDataKey, identitySecretName, image, apiHostname, pullSecretName string) *batchv1.Job { labels := CommonLabels(instance) backoffLimit := int32(2) @@ -121,7 +121,7 @@ func BuildCRCAgentJob(instance *brokerv1alpha1.ClusterInstance, vmIP, sshKeySecr return &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ - Name: CRCAgentJobName(instance.Name), + Name: CRCAgentJobName(instance.Name, vmiUID), Namespace: instance.Namespace, Labels: labels, }, @@ -143,13 +143,16 @@ func BuildCRCAgentJob(instance *brokerv1alpha1.ClusterInstance, vmIP, sshKeySecr {Name: "INSTANCE_NAME", Value: instance.Name}, {Name: "INSTANCE_NAMESPACE", Value: instance.Namespace}, {Name: "CRC_SSH_HOST", Value: vmIP}, + {Name: "CRC_VMI_UID", Value: vmiUID}, {Name: "CRC_SSH_KEY_PATH", Value: CRCAgentSSHKeyPath()}, + {Name: "CRC_IDENTITY_PATH", Value: "/etc/crc-agent/identity"}, {Name: "PULL_SECRET_PATH", Value: CRCAgentPullSecretPath()}, {Name: CRCAPIHostnameEnvVar, Value: apiHostname}, }, VolumeMounts: []corev1.VolumeMount{ {Name: "pull-secret", MountPath: crcAgentPullSecretMountPath, ReadOnly: true}, {Name: "bundle-ssh-key", MountPath: crcAgentSSHKeyMountPath, ReadOnly: true}, + {Name: "identity", MountPath: "/etc/crc-agent/identity", ReadOnly: true}, }, }, }, @@ -173,6 +176,10 @@ func BuildCRCAgentJob(instance *brokerv1alpha1.ClusterInstance, vmIP, sshKeySecr }, }, }, + { + Name: "identity", + VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: identitySecretName}}, + }, }, }, }, diff --git a/internal/resources/crcagent_test.go b/internal/resources/crcagent_test.go new file mode 100644 index 0000000..c53d17f --- /dev/null +++ b/internal/resources/crcagent_test.go @@ -0,0 +1,38 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "strings" + "testing" +) + +func TestCRCAgentJobNameIsVMIScopedAndBounded(t *testing.T) { + instanceName := strings.Repeat("a", 63) + first := CRCAgentJobName(instanceName, "first-vmi") + second := CRCAgentJobName(instanceName, "second-vmi") + + if len(first) > 63 { + t.Fatalf("job name length = %d, want at most 63", len(first)) + } + if first == second { + t.Fatalf("job names for different VMIs must differ: %q", first) + } + if first != CRCAgentJobName(instanceName, "first-vmi") { + t.Fatalf("job name must be deterministic") + } +} From 3a2f05aa0e7fe88c81ca947b2ecc5ede6a3bfa9e Mon Sep 17 00:00:00 2001 From: Caleb Xu Date: Fri, 4 Sep 2026 12:48:48 -0400 Subject: [PATCH 4/5] fix(crc): recover ready instances after VMI replacement Signed-off-by: Caleb Xu Assisted-by: OpenCode (GPT-5.6 Terra) --- .../clusterinstance_crc_recovery_test.go | 450 ++++++++++++++++++ 1 file changed, 450 insertions(+) create mode 100644 internal/controller/clusterinstance_crc_recovery_test.go diff --git a/internal/controller/clusterinstance_crc_recovery_test.go b/internal/controller/clusterinstance_crc_recovery_test.go new file mode 100644 index 0000000..9ac904a --- /dev/null +++ b/internal/controller/clusterinstance_crc_recovery_test.go @@ -0,0 +1,450 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + kubevirtv1 "kubevirt.io/api/core/v1" + cdiv1beta1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + routev1 "github.com/openshift/api/route/v1" + + brokerv1alpha1 "github.com/caxu-rh/guestcluster-operator/api/v1alpha1" + "github.com/caxu-rh/guestcluster-operator/internal/resources" +) + +const ( + recoveryInstanceName = "crc-recovery" + oldAPIEndpoint = "https://old.example.test" +) + +func TestCRCVMIDChanged(t *testing.T) { + const oldUID = "old" + tests := []struct { + name string + previous string + current string + changed bool + }{ + {name: "same UID", previous: oldUID, current: oldUID, changed: false}, + {name: "replacement", previous: oldUID, current: "new", changed: true}, + {name: "unrecorded UID", current: "new", changed: true}, + {name: "missing current UID", previous: oldUID, changed: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := crcVMIChanged(tt.previous, tt.current); got != tt.changed { + t.Fatalf("crcVMIChanged(%q, %q) = %t, want %t", tt.previous, tt.current, got, tt.changed) + } + }) + } +} + +func TestReconcileReadyCRC_VMIReplacementRemovesPreviousHandoff(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + Status: brokerv1alpha1.ClusterInstanceStatus{ + Phase: brokerv1alpha1.PhaseReady, + APIEndpoint: oldAPIEndpoint, + KubeconfigSecretRef: corev1.LocalObjectReference{Name: resources.KubeconfigSecretName(recoveryInstanceName)}, + CRC: &brokerv1alpha1.CRCBackingStatus{VMName: recoveryInstanceName, DataVolumeName: recoveryInstanceName + "-rootdisk", VMIUID: "old-vmi"}, + }, + } + job := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: resources.CRCAgentJobName(instance.Name, "old-vmi"), Namespace: instance.Namespace}} + raw := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "old-vmi"), Namespace: instance.Namespace}} + published := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: resources.KubeconfigSecretName(instance.Name), Namespace: instance.Namespace}} + identity := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: resources.CRCIdentitySecretName(instance.Name), Namespace: instance.Namespace}} + vmi := &kubevirtv1.VirtualMachineInstance{ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("new-vmi")}} + c := newCRCRecoveryFakeClient(t, instance, job, raw, published, identity, vmi) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + if _, err := r.reconcileReadyCRC(ctx, instance); err != nil { + t.Fatalf("reconcileReadyCRC: %v", err) + } + if err := c.Get(ctx, client.ObjectKeyFromObject(identity), identity); err != nil { + t.Fatalf("expected CRC identity to survive VMI replacement: %v", err) + } + for _, obj := range []client.Object{job, published} { + if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err == nil { + t.Fatalf("expected %T %s to be deleted", obj, client.ObjectKeyFromObject(obj)) + } + } + if err := c.Get(ctx, client.ObjectKeyFromObject(raw), raw); err != nil { + t.Fatalf("expected retained raw handoff: %v", err) + } + + got := &brokerv1alpha1.ClusterInstance{} + if err := c.Get(ctx, client.ObjectKeyFromObject(instance), got); err != nil { + t.Fatalf("getting instance: %v", err) + } + if got.Status.Phase != brokerv1alpha1.PhaseProvisioning || got.Status.CRC.VMIUID != "new-vmi" { + t.Fatalf("expected Provisioning with new VMI UID, got %+v", got.Status) + } + if got.Status.APIEndpoint != "" || got.Status.KubeconfigSecretRef.Name != "" { + t.Fatalf("expected cleared published access details, got %+v", got.Status) + } +} + +func TestReconcileReadyCRCRequeuesHealthCheck(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + if request.URL.Path != "/readyz" { + t.Errorf("request path = %q, want /readyz", request.URL.Path) + } + response.WriteHeader(http.StatusOK) + })) + defer server.Close() + + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "crc-ready", Namespace: testNamespace}, + Status: brokerv1alpha1.ClusterInstanceStatus{ + Phase: brokerv1alpha1.PhaseReady, + CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: "vmi-uid"}, + }, + } + vmi := &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("vmi-uid")}, + Status: kubevirtv1.VirtualMachineInstanceStatus{Phase: kubevirtv1.Running}, + } + published := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: resources.KubeconfigSecretName(instance.Name), Namespace: instance.Namespace}, + Data: map[string][]byte{resources.KubeconfigSecretKey: []byte(fmt.Sprintf(`apiVersion: v1 +kind: Config +clusters: +- cluster: + server: %s + insecure-skip-tls-verify: true + name: guest +contexts: +- context: + cluster: guest + name: guest +current-context: guest +`, server.URL))}, + } + c := newCRCRecoveryFakeClient(t, instance, vmi, published) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + result, err := r.reconcileReadyCRC(context.Background(), instance) + if err != nil { + t.Fatalf("reconcileReadyCRC: %v", err) + } + if result.RequeueAfter != crcReadyRequeueInterval { + t.Fatalf("RequeueAfter = %s, want %s", result.RequeueAfter, crcReadyRequeueInterval) + } +} + +func TestReconcileReadyCRCRemovesLeaseEligibilityWhenHealthCheckFails(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + Status: brokerv1alpha1.ClusterInstanceStatus{ + Phase: brokerv1alpha1.PhaseReady, + APIEndpoint: oldAPIEndpoint, + KubeconfigSecretRef: corev1.LocalObjectReference{Name: resources.KubeconfigSecretName(recoveryInstanceName)}, + CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: "vmi-uid"}, + }, + } + vmi := &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("vmi-uid")}, + Status: kubevirtv1.VirtualMachineInstanceStatus{Phase: kubevirtv1.Running}, + } + published := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: resources.KubeconfigSecretName(instance.Name), Namespace: instance.Namespace}, + Data: map[string][]byte{resources.KubeconfigSecretKey: []byte("invalid")}, + } + raw := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "vmi-uid"), Namespace: instance.Namespace}, + Data: map[string][]byte{resources.KubeconfigSecretKey: []byte("retained")}, + } + c := newCRCRecoveryFakeClient(t, instance, vmi, published, raw) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + result, err := r.reconcileReadyCRC(ctx, instance) + if err != nil { + t.Fatalf("reconcileReadyCRC: %v", err) + } + if result.RequeueAfter != requeueInterval { + t.Fatalf("RequeueAfter = %s, want %s", result.RequeueAfter, requeueInterval) + } + for _, obj := range []client.Object{published, raw} { + if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { + t.Fatalf("expected %T to be retained: %v", obj, err) + } + } + got := &brokerv1alpha1.ClusterInstance{} + if err := c.Get(ctx, client.ObjectKeyFromObject(instance), got); err != nil { + t.Fatalf("getting instance: %v", err) + } + if got.Status.Phase != brokerv1alpha1.PhaseProvisioning { + t.Fatalf("phase = %s, want Provisioning", got.Status.Phase) + } + if got.Status.KubeconfigSecretRef.Name != "" { + t.Fatalf("kubeconfig reference = %q, want empty", got.Status.KubeconfigSecretRef.Name) + } + readyCondition := apimeta.FindStatusCondition(got.Status.Conditions, conditionTypeReady) + if readyCondition == nil || readyCondition.Status != metav1.ConditionFalse { + t.Fatalf("expected Ready=False, got %+v", readyCondition) + } + condition := apimeta.FindStatusCondition(got.Status.Conditions, conditionTypeGuestAPIReachable) + if condition == nil || condition.Status != metav1.ConditionFalse { + t.Fatalf("expected GuestAPIReachable=False, got %+v", condition) + } +} + +func TestReconcileReadyCRCDoesNotRestoreKubeconfigBeforeHealthCheck(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + Status: brokerv1alpha1.ClusterInstanceStatus{ + Phase: brokerv1alpha1.PhaseReady, + APIEndpoint: oldAPIEndpoint, + KubeconfigSecretRef: corev1.LocalObjectReference{Name: resources.KubeconfigSecretName(recoveryInstanceName)}, + CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: "vmi-uid"}, + }, + } + vmi := &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("vmi-uid")}, + Status: kubevirtv1.VirtualMachineInstanceStatus{Phase: kubevirtv1.Running}, + } + raw := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "vmi-uid"), Namespace: instance.Namespace}, + Data: map[string][]byte{ + resources.KubeconfigSecretKey: []byte("invalid"), + resources.VMIUIDSecretKey: []byte("vmi-uid"), + }, + } + c := newCRCRecoveryFakeClient(t, instance, vmi, raw) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + result, err := r.reconcileReadyCRC(ctx, instance) + if err != nil { + t.Fatalf("reconcileReadyCRC: %v", err) + } + if result.RequeueAfter != requeueInterval { + t.Fatalf("RequeueAfter = %s, want %s", result.RequeueAfter, requeueInterval) + } + if err := c.Get(ctx, client.ObjectKeyFromObject(raw), raw); err != nil { + t.Fatalf("expected raw handoff to be retained: %v", err) + } + published := &corev1.Secret{} + publishedKey := types.NamespacedName{Name: resources.KubeconfigSecretName(instance.Name), Namespace: instance.Namespace} + if err := c.Get(ctx, publishedKey, published); err == nil { + t.Fatalf("expected published kubeconfig to remain absent") + } + got := &brokerv1alpha1.ClusterInstance{} + if err := c.Get(ctx, client.ObjectKeyFromObject(instance), got); err != nil { + t.Fatalf("getting instance: %v", err) + } + if got.Status.Phase != brokerv1alpha1.PhaseProvisioning { + t.Fatalf("phase = %s, want Provisioning", got.Status.Phase) + } + if got.Status.KubeconfigSecretRef.Name != "" { + t.Fatalf("kubeconfig reference = %q, want empty", got.Status.KubeconfigSecretRef.Name) + } + for _, conditionType := range []string{conditionTypeReady, conditionTypeGuestAPIReachable} { + condition := apimeta.FindStatusCondition(got.Status.Conditions, conditionType) + if condition == nil || condition.Status != metav1.ConditionFalse { + t.Fatalf("expected %s=False, got %+v", conditionType, condition) + } + } +} + +func TestTeardownCRCBackingDeletesAllVMIHandoffs(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + } + oldJob := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{ + Name: resources.CRCAgentJobName(instance.Name, "old-vmi"), + Namespace: instance.Namespace, + Labels: resources.CommonLabels(instance), + }} + currentJob := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{ + Name: resources.CRCAgentJobName(instance.Name, "current-vmi"), + Namespace: instance.Namespace, + Labels: resources.CommonLabels(instance), + }} + rawLabels := map[string]string{resources.LabelManagedBy: "crc-agent", resources.LabelInstance: instance.Name} + oldRaw := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "old-vmi"), + Namespace: instance.Namespace, + Labels: rawLabels, + }} + currentRaw := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "current-vmi"), + Namespace: instance.Namespace, + Labels: rawLabels, + }} + c := newCRCRecoveryFakeClient(t, instance, oldJob, currentJob, oldRaw, currentRaw) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + if err := r.teardownCRCBacking(ctx, instance); err != nil { + t.Fatalf("teardownCRCBacking: %v", err) + } + for _, obj := range []client.Object{oldJob, currentJob, oldRaw, currentRaw} { + if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err == nil { + t.Fatalf("expected %T %s to be deleted", obj, client.ObjectKeyFromObject(obj)) + } + } +} + +func TestReconcileProvisioningCRCVMI_VMIReplacementRemovesPreviousHandoff(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + Status: brokerv1alpha1.ClusterInstanceStatus{ + Phase: brokerv1alpha1.PhaseProvisioning, + APIEndpoint: oldAPIEndpoint, + KubeconfigSecretRef: corev1.LocalObjectReference{Name: resources.KubeconfigSecretName(recoveryInstanceName)}, + CRC: &brokerv1alpha1.CRCBackingStatus{VMName: recoveryInstanceName, DataVolumeName: recoveryInstanceName + "-rootdisk", VMIUID: "old-vmi"}, + }, + } + job := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: resources.CRCAgentJobName(instance.Name, "old-vmi"), Namespace: instance.Namespace}} + raw := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "old-vmi"), Namespace: instance.Namespace}} + published := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: resources.KubeconfigSecretName(instance.Name), Namespace: instance.Namespace}} + identity := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: resources.CRCIdentitySecretName(instance.Name), Namespace: instance.Namespace}} + vmi := &kubevirtv1.VirtualMachineInstance{ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("new-vmi")}} + c := newCRCRecoveryFakeClient(t, instance, job, raw, published, identity, vmi) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + result, err := r.reconcileProvisioningCRCVMI(ctx, instance) + if err != nil { + t.Fatalf("reconcileProvisioningCRCVMI: %v", err) + } + if err := c.Get(ctx, client.ObjectKeyFromObject(identity), identity); err != nil { + t.Fatalf("expected CRC identity to survive VMI replacement: %v", err) + } + if result == nil || result.RequeueAfter != requeueInterval { + t.Fatalf("expected a recovery requeue, got %+v", result) + } + for _, obj := range []client.Object{job, published} { + if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err == nil { + t.Fatalf("expected %T %s to be deleted", obj, client.ObjectKeyFromObject(obj)) + } + } + if err := c.Get(ctx, client.ObjectKeyFromObject(raw), raw); err != nil { + t.Fatalf("expected retained raw handoff: %v", err) + } + + got := &brokerv1alpha1.ClusterInstance{} + if err := c.Get(ctx, client.ObjectKeyFromObject(instance), got); err != nil { + t.Fatalf("getting instance: %v", err) + } + if got.Status.Phase != brokerv1alpha1.PhaseProvisioning || got.Status.CRC.VMIUID != "new-vmi" { + t.Fatalf("expected Provisioning with new VMI UID, got %+v", got.Status) + } +} + +func TestReconcileProvisioningCRCVMI_UnrecordedVMIUIDDoesNotInvalidateHandoff(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + Status: brokerv1alpha1.ClusterInstanceStatus{ + Phase: brokerv1alpha1.PhaseProvisioning, + CRC: &brokerv1alpha1.CRCBackingStatus{VMName: recoveryInstanceName, DataVolumeName: recoveryInstanceName + "-rootdisk"}, + }, + } + job := &batchv1.Job{ObjectMeta: metav1.ObjectMeta{Name: resources.CRCAgentJobName(instance.Name, "vmi"), Namespace: instance.Namespace}} + vmi := &kubevirtv1.VirtualMachineInstance{ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("vmi")}} + c := newCRCRecoveryFakeClient(t, instance, job, vmi) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + result, err := r.reconcileProvisioningCRCVMI(ctx, instance) + if err != nil { + t.Fatalf("reconcileProvisioningCRCVMI: %v", err) + } + if result != nil { + t.Fatalf("expected normal provisioning to continue, got %+v", result) + } + if err := c.Get(ctx, client.ObjectKeyFromObject(job), job); err != nil { + t.Fatalf("expected crc-agent Job to be preserved: %v", err) + } +} + +func TestCheckCRCKubeconfigHandoffRejectsDifferentVMI(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + Status: brokerv1alpha1.ClusterInstanceStatus{ + CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: "current-vmi"}, + }, + } + raw := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "current-vmi"), Namespace: instance.Namespace}, + Data: map[string][]byte{ + resources.KubeconfigSecretKey: []byte("stale-kubeconfig"), + resources.VMIUIDSecretKey: []byte("old-vmi"), + }, + } + c := newCRCRecoveryFakeClient(t, instance, raw) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + kubeconfig, _, err := r.checkCRCKubeconfigHandoff(ctx, instance) + if err != nil { + t.Fatalf("checkCRCKubeconfigHandoff: %v", err) + } + if kubeconfig != nil { + t.Fatalf("kubeconfig = %q, want stale handoff rejected", kubeconfig) + } +} + +func newCRCRecoveryFakeClient(t *testing.T, objects ...client.Object) client.Client { + t.Helper() + s := runtime.NewScheme() + if err := scheme.AddToScheme(s); err != nil { + t.Fatalf("adding core scheme: %v", err) + } + if err := brokerv1alpha1.AddToScheme(s); err != nil { + t.Fatalf("adding broker scheme: %v", err) + } + if err := kubevirtv1.AddToScheme(s); err != nil { + t.Fatalf("adding KubeVirt scheme: %v", err) + } + if err := cdiv1beta1.AddToScheme(s); err != nil { + t.Fatalf("adding CDI scheme: %v", err) + } + if err := routev1.AddToScheme(s); err != nil { + t.Fatalf("adding OpenShift Route scheme: %v", err) + } + return fake.NewClientBuilder(). + WithScheme(s). + WithStatusSubresource(&brokerv1alpha1.ClusterInstance{}). + WithIndex(&brokerv1alpha1.ClusterLease{}, leaseInstanceRefIndexField, func(obj client.Object) []string { + lease, ok := obj.(*brokerv1alpha1.ClusterLease) + if !ok || lease.Status.InstanceRef == nil { + return nil + } + return []string{lease.Status.InstanceRef.Name} + }). + WithObjects(objects...). + Build() +} From 74ed144c243a425f74753aa74e2a14367253359d Mon Sep 17 00:00:00 2001 From: Caleb Xu Date: Fri, 4 Sep 2026 12:48:51 -0400 Subject: [PATCH 5/5] test(e2e): cover CRC VMI replacement recovery Signed-off-by: Caleb Xu Assisted-by: OpenCode (GPT-5.6 Terra) --- .../clusterinstance_crc_recovery_test.go | 19 +- test/e2e/e2e_test.go | 243 +++++++++++++++++- test/e2e/testdata/vmi-crd.yaml | 87 +++++++ 3 files changed, 337 insertions(+), 12 deletions(-) create mode 100644 test/e2e/testdata/vmi-crd.yaml diff --git a/internal/controller/clusterinstance_crc_recovery_test.go b/internal/controller/clusterinstance_crc_recovery_test.go index 9ac904a..1118132 100644 --- a/internal/controller/clusterinstance_crc_recovery_test.go +++ b/internal/controller/clusterinstance_crc_recovery_test.go @@ -44,6 +44,7 @@ import ( const ( recoveryInstanceName = "crc-recovery" oldAPIEndpoint = "https://old.example.test" + recoveryVMIUID = "vmi-uid" ) func TestCRCVMIDChanged(t *testing.T) { @@ -127,11 +128,11 @@ func TestReconcileReadyCRCRequeuesHealthCheck(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "crc-ready", Namespace: testNamespace}, Status: brokerv1alpha1.ClusterInstanceStatus{ Phase: brokerv1alpha1.PhaseReady, - CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: "vmi-uid"}, + CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: recoveryVMIUID}, }, } vmi := &kubevirtv1.VirtualMachineInstance{ - ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("vmi-uid")}, + ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID(recoveryVMIUID)}, Status: kubevirtv1.VirtualMachineInstanceStatus{Phase: kubevirtv1.Running}, } published := &corev1.Secret{ @@ -170,11 +171,11 @@ func TestReconcileReadyCRCRemovesLeaseEligibilityWhenHealthCheckFails(t *testing Phase: brokerv1alpha1.PhaseReady, APIEndpoint: oldAPIEndpoint, KubeconfigSecretRef: corev1.LocalObjectReference{Name: resources.KubeconfigSecretName(recoveryInstanceName)}, - CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: "vmi-uid"}, + CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: recoveryVMIUID}, }, } vmi := &kubevirtv1.VirtualMachineInstance{ - ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("vmi-uid")}, + ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID(recoveryVMIUID)}, Status: kubevirtv1.VirtualMachineInstanceStatus{Phase: kubevirtv1.Running}, } published := &corev1.Secret{ @@ -182,7 +183,7 @@ func TestReconcileReadyCRCRemovesLeaseEligibilityWhenHealthCheckFails(t *testing Data: map[string][]byte{resources.KubeconfigSecretKey: []byte("invalid")}, } raw := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "vmi-uid"), Namespace: instance.Namespace}, + ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, recoveryVMIUID), Namespace: instance.Namespace}, Data: map[string][]byte{resources.KubeconfigSecretKey: []byte("retained")}, } c := newCRCRecoveryFakeClient(t, instance, vmi, published, raw) @@ -228,18 +229,18 @@ func TestReconcileReadyCRCDoesNotRestoreKubeconfigBeforeHealthCheck(t *testing.T Phase: brokerv1alpha1.PhaseReady, APIEndpoint: oldAPIEndpoint, KubeconfigSecretRef: corev1.LocalObjectReference{Name: resources.KubeconfigSecretName(recoveryInstanceName)}, - CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: "vmi-uid"}, + CRC: &brokerv1alpha1.CRCBackingStatus{VMIUID: recoveryVMIUID}, }, } vmi := &kubevirtv1.VirtualMachineInstance{ - ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID("vmi-uid")}, + ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID(recoveryVMIUID)}, Status: kubevirtv1.VirtualMachineInstanceStatus{Phase: kubevirtv1.Running}, } raw := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, "vmi-uid"), Namespace: instance.Namespace}, + ObjectMeta: metav1.ObjectMeta{Name: resources.RawKubeconfigSecretNameForVMI(instance.Name, recoveryVMIUID), Namespace: instance.Namespace}, Data: map[string][]byte{ resources.KubeconfigSecretKey: []byte("invalid"), - resources.VMIUIDSecretKey: []byte("vmi-uid"), + resources.VMIUIDSecretKey: []byte(recoveryVMIUID), }, } c := newCRCRecoveryFakeClient(t, instance, vmi, raw) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 2a374f8..3fec5a6 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -17,6 +17,7 @@ limitations under the License. package e2e import ( + "encoding/base64" "encoding/json" "fmt" "os" @@ -49,6 +50,10 @@ const metricsRoleBindingName = "guestcluster-operator-metrics-binding" // config/openshift-config-rbac. See ClusterInstanceReconciler.resolvePullSecret. const openshiftConfigNamespace = "openshift-config" +const crcRecoveryNamespace = "crc-recovery-e2e" + +const crcRecoveryInstanceName = "crc-vmi-recovery" + var _ = Describe("Manager", Ordered, func() { var controllerPodName string @@ -87,19 +92,36 @@ var _ = Describe("Manager", Ordered, func() { _, err = utils.Run(cmd) Expect(err).NotTo(HaveOccurred(), "Failed to install CRDs") + By("installing synthetic CRC backing CRDs for recovery tests") + cmd = exec.Command("kubectl", "apply", "-f", "test/e2e/testdata/vmi-crd.yaml") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred(), "Failed to install VirtualMachineInstance CRD") + By("deploying the controller-manager") cmd = exec.Command("make", "deploy", fmt.Sprintf("IMG=%s", projectImage)) _, err = utils.Run(cmd) Expect(err).NotTo(HaveOccurred(), "Failed to deploy the controller-manager") }) - // After all tests have been executed, clean up by undeploying the controller, uninstalling CRDs, - // and deleting the namespace. + // Remove custom resources while the controller is still running. CRD deletion + // waits for custom-resource finalizers. AfterAll(func() { By("cleaning up the curl pod for metrics") cmd := exec.Command("kubectl", "delete", "pod", "curl-metrics", "-n", namespace) _, _ = utils.Run(cmd) + By("removing the CRC recovery instance before undeploying the controller-manager") + cmd = exec.Command("kubectl", "delete", "clusterinstance", crcRecoveryInstanceName, + "-n", crcRecoveryNamespace, "--ignore-not-found", "--wait=true", "--timeout=2m") + _, err := utils.Run(cmd) + Expect(err).NotTo(HaveOccurred(), "Failed to remove CRC recovery instance") + + By("removing the CRC recovery namespace before uninstalling CRDs") + cmd = exec.Command("kubectl", "delete", "namespace", crcRecoveryNamespace, + "--ignore-not-found", "--wait=true", "--timeout=2m") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred(), "Failed to remove CRC recovery namespace") + By("undeploying the controller-manager") cmd = exec.Command("make", "undeploy") _, _ = utils.Run(cmd) @@ -108,6 +130,10 @@ var _ = Describe("Manager", Ordered, func() { cmd = exec.Command("make", "uninstall") _, _ = utils.Run(cmd) + By("removing synthetic CRC backing CRDs") + cmd = exec.Command("kubectl", "delete", "-f", "test/e2e/testdata/vmi-crd.yaml", "--ignore-not-found") + _, _ = utils.Run(cmd) + By("removing manager namespace") cmd = exec.Command("kubectl", "delete", "ns", namespace) _, _ = utils.Run(cmd) @@ -124,8 +150,21 @@ var _ = Describe("Manager", Ordered, func() { AfterEach(func() { specReport := CurrentSpecReport() if specReport.Failed() { + // The recovery test restarts the controller, so refresh its pod name + // before collecting diagnostics. + cmd := exec.Command("kubectl", "get", "pods", "-l", "control-plane=controller-manager", + "-o", "go-template={{ range .items }}{{ if not .metadata.deletionTimestamp }}"+ + "{{ .metadata.name }}{{ \"\\n\" }}{{ end }}{{ end }}", + "-n", namespace) + if podOutput, err := utils.Run(cmd); err == nil { + podNames := utils.GetNonEmptyLines(podOutput) + if len(podNames) == 1 { + controllerPodName = podNames[0] + } + } + By("Fetching controller manager pod logs") - cmd := exec.Command("kubectl", "logs", controllerPodName, "-n", namespace) + cmd = exec.Command("kubectl", "logs", controllerPodName, "-n", namespace) controllerLogs, err := utils.Run(cmd) if err == nil { _, _ = fmt.Fprintf(GinkgoWriter, "Controller logs:\n %s", controllerLogs) @@ -284,6 +323,150 @@ var _ = Describe("Manager", Ordered, func() { )) }) + It("should invalidate CRC handoff after VMI replacement", func() { + By("creating an isolated namespace and readiness identity") + cmd := exec.Command("kubectl", "create", "namespace", crcRecoveryNamespace) + _, err := utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + cmd = exec.Command("kubectl", "create", "serviceaccount", "crc-readyz", "-n", crcRecoveryNamespace) + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + cmd = exec.Command("kubectl", "create", "clusterrole", "crc-readyz", "--verb=get", "--non-resource-url=/readyz") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(deleteResource, "clusterrole", "crc-readyz") + cmd = exec.Command("kubectl", "create", "clusterrolebinding", "crc-readyz", + "--clusterrole=crc-readyz", fmt.Sprintf("--serviceaccount=%s:crc-readyz", crcRecoveryNamespace)) + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(deleteResource, "clusterrolebinding", "crc-readyz") + cmd = exec.Command("kubectl", "create", "token", "crc-readyz", "-n", crcRecoveryNamespace) + token, err := utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + By("stopping the controller while creating the synthetic Ready CRC instance") + cmd = exec.Command("kubectl", "scale", "deployment", + "guestcluster-operator-controller-manager", "-n", namespace, "--replicas=0") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + cmd = exec.Command("kubectl", "rollout", "status", + "deployment/guestcluster-operator-controller-manager", "-n", namespace, "--timeout=2m") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func() { + cmd := exec.Command("kubectl", "scale", "deployment", + "guestcluster-operator-controller-manager", "-n", namespace, "--replicas=1") + _, _ = utils.Run(cmd) + }) + + By("creating a Ready CRC instance with its first VMI") + instanceName := crcRecoveryInstanceName + Expect(applyManifest(fmt.Sprintf(` +apiVersion: kubevirt.io/v1 +kind: VirtualMachineInstance +metadata: + name: %[1]s + namespace: %[2]s +spec: {} +--- +apiVersion: v1 +kind: Secret +metadata: + name: %[1]s-pull-secret + namespace: %[2]s +type: kubernetes.io/dockerconfigjson +data: + .dockerconfigjson: e30= +--- +apiVersion: v1 +kind: Secret +metadata: + name: %[1]s-bundle-ssh-key + namespace: %[2]s +data: + id_ecdsa: dGVzdA== +--- +apiVersion: v1 +kind: Secret +metadata: + name: %[1]s-kubeconfig + namespace: %[2]s +data: + kubeconfig: %s +--- +apiVersion: guestcluster.opdev.io/v1alpha1 +kind: ClusterInstance +metadata: + name: %[1]s + namespace: %[2]s +spec: + type: crc + template: + ocpVersion: "4.16.0" + memory: 16Gi + cores: 4 + rootVolumeSize: 80Gi + releaseImage: https://example.test/crc.qcow2 + pullSecretRef: + name: %[1]s-pull-secret + bundleSSHKeyRef: + name: %[1]s-bundle-ssh-key +`, instanceName, crcRecoveryNamespace, + base64.StdEncoding.EncodeToString([]byte(readyzKubeconfig(token)))))).To(Succeed()) + + oldVMIUID := resourceField("virtualmachineinstance", instanceName, "{.metadata.uid}") + Expect(oldVMIUID).NotTo(BeEmpty()) + vmiStatus := `{"status":{"phase":"Running"}}` + cmd = exec.Command("kubectl", "patch", "virtualmachineinstance", instanceName, "-n", + crcRecoveryNamespace, "--subresource=status", "--type=merge", "-p", vmiStatus) + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + status := fmt.Sprintf(`{"status":{"phase":"Ready","apiEndpoint":"https://kubernetes.default.svc", +"kubeconfigSecretRef":{"name":"%[1]s-kubeconfig"},"crc":{"vmName":"%[1]s", +"dataVolumeName":"%[1]s-rootdisk","vmiUID":"%[2]s"}}}`, instanceName, oldVMIUID) + cmd = exec.Command("kubectl", "patch", "clusterinstance", instanceName, "-n", + crcRecoveryNamespace, "--subresource=status", "--type=merge", "-p", status) + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + By("resuming the controller") + cmd = exec.Command("kubectl", "scale", "deployment", + "guestcluster-operator-controller-manager", "-n", namespace, "--replicas=1") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + cmd = exec.Command("kubectl", "rollout", "status", + "deployment/guestcluster-operator-controller-manager", "-n", namespace, "--timeout=2m") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + By("replacing the VMI") + cmd = exec.Command("kubectl", "delete", "virtualmachineinstance", instanceName, + "-n", crcRecoveryNamespace, "--wait=true") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + By("creating the replacement VMI") + manifest := fmt.Sprintf("apiVersion: kubevirt.io/v1\nkind: VirtualMachineInstance\nmetadata:\n"+ + " name: %s\n namespace: %s\nspec: {}\n", instanceName, crcRecoveryNamespace) + Expect(applyManifest(manifest)).To(Succeed()) + Expect(resourceField("virtualmachineinstance", instanceName, "{.metadata.uid}")). + NotTo(Equal(oldVMIUID)) + cmd = exec.Command("kubectl", "patch", "virtualmachineinstance", instanceName, "-n", + crcRecoveryNamespace, "--subresource=status", "--type=merge", "-p", vmiStatus) + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for stale CRC handoff state to be removed") + Eventually(func(g Gomega) { + g.Expect(resourceField("clusterinstance", instanceName, "{.status.phase}")). + To(Equal("Provisioning")) + g.Expect(resourceField("clusterinstance", instanceName, "{.status.kubeconfigSecretRef.name}")). + To(BeEmpty()) + g.Expect(resourceExists("secret", instanceName+"-kubeconfig", crcRecoveryNamespace)).To(BeFalse()) + }).Should(Succeed()) + }) + // +kubebuilder:scaffold:e2e-webhooks-checks // TODO: Customize the e2e test suite with scenarios specific to your project. @@ -348,6 +531,60 @@ func getMetricsOutput() string { return metricsOutput } +func applyManifest(manifest string) error { + file, err := os.CreateTemp("", "guestcluster-e2e-*.yaml") + if err != nil { + return err + } + defer func() { _ = os.Remove(file.Name()) }() + if _, err := file.WriteString(manifest); err != nil { + return err + } + if err := file.Close(); err != nil { + return err + } + _, err = utils.Run(exec.Command("kubectl", "apply", "-f", file.Name())) + return err +} + +func resourceField(resource, name, jsonPath string) string { + cmd := exec.Command("kubectl", "get", resource, name, "-n", crcRecoveryNamespace, "-o", "jsonpath="+jsonPath) + output, err := utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + return output +} + +func resourceExists(resource, name, namespace string) bool { + cmd := exec.Command("kubectl", "get", resource, name, "-n", namespace) + _, err := utils.Run(cmd) + return err == nil +} + +func deleteResource(resource, name string) { + _, _ = utils.Run(exec.Command("kubectl", "delete", resource, name, "--ignore-not-found")) +} + +func readyzKubeconfig(token string) string { + return fmt.Sprintf(`apiVersion: v1 +kind: Config +clusters: +- cluster: + server: https://kubernetes.default.svc + insecure-skip-tls-verify: true + name: management +contexts: +- context: + cluster: management + user: readyz + name: management +current-context: management +users: +- name: readyz + user: + token: %s +`, token) +} + // tokenRequest is a simplified representation of the Kubernetes TokenRequest API response, // containing only the token field that we need to extract. type tokenRequest struct { diff --git a/test/e2e/testdata/vmi-crd.yaml b/test/e2e/testdata/vmi-crd.yaml new file mode 100644 index 0000000..9c8e353 --- /dev/null +++ b/test/e2e/testdata/vmi-crd.yaml @@ -0,0 +1,87 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: virtualmachineinstances.kubevirt.io +spec: + group: kubevirt.io + names: + kind: VirtualMachineInstance + listKind: VirtualMachineInstanceList + plural: virtualmachineinstances + singular: virtualmachineinstance + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + subresources: + status: {} + schema: + openAPIV3Schema: + type: object + x-kubernetes-preserve-unknown-fields: true +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: virtualmachines.kubevirt.io +spec: + group: kubevirt.io + names: + kind: VirtualMachine + listKind: VirtualMachineList + plural: virtualmachines + singular: virtualmachine + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + subresources: + status: {} + schema: + openAPIV3Schema: + type: object + x-kubernetes-preserve-unknown-fields: true +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: datavolumes.cdi.kubevirt.io +spec: + group: cdi.kubevirt.io + names: + kind: DataVolume + listKind: DataVolumeList + plural: datavolumes + singular: datavolume + scope: Namespaced + versions: + - name: v1beta1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + x-kubernetes-preserve-unknown-fields: true +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: routes.route.openshift.io +spec: + group: route.openshift.io + names: + kind: Route + listKind: RouteList + plural: routes + singular: route + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + x-kubernetes-preserve-unknown-fields: true