diff --git a/internal/controller/clusterinstance_crc.go b/internal/controller/clusterinstance_crc.go index 4470fbb..42fdf26 100644 --- a/internal/controller/clusterinstance_crc.go +++ b/internal/controller/clusterinstance_crc.go @@ -230,19 +230,22 @@ func (r *ClusterInstanceReconciler) ensureCRCBacking(ctx context.Context, instan // 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. + // idempotent: once created, the Job runs to completion or reaches a terminal + // failure condition on its own. 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) { + existingJob := &batchv1.Job{} + if err := r.Get(ctx, types.NamespacedName{Name: job.Name, Namespace: job.Namespace}, existingJob); 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) } log.Info("created crc-agent Job", "job", job.Name, "vmIP", vmIP) } else if err != nil { return res, fmt.Errorf("getting crc-agent Job %s/%s: %w", job.Namespace, job.Name, err) + } else if err := checkCRCAgentJobFailure(existingJob); err != nil { + return res, err } // Once the crc-agent Job completes successfully, it publishes the raw @@ -266,6 +269,15 @@ func (r *ClusterInstanceReconciler) ensureCRCBacking(ctx context.Context, instan return res, nil } +func checkCRCAgentJobFailure(job *batchv1.Job) error { + for _, condition := range job.Status.Conditions { + if condition.Type == batchv1.JobFailed && condition.Status == corev1.ConditionTrue { + return fmt.Errorf("crc-agent Job %s failed: %s", job.Name, condition.Reason) + } + } + return 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) { diff --git a/internal/controller/clusterinstance_crc_recovery_test.go b/internal/controller/clusterinstance_crc_recovery_test.go index 1118132..e052320 100644 --- a/internal/controller/clusterinstance_crc_recovery_test.go +++ b/internal/controller/clusterinstance_crc_recovery_test.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + configv1 "github.com/openshift/api/config/v1" routev1 "github.com/openshift/api/route/v1" brokerv1alpha1 "github.com/caxu-rh/guestcluster-operator/api/v1alpha1" @@ -418,6 +419,68 @@ func TestCheckCRCKubeconfigHandoffRejectsDifferentVMI(t *testing.T) { } } +func TestReconcileCRC_FailsWhenAgentJobIsTerminal(t *testing.T) { + for _, reason := range []string{batchv1.JobReasonBackoffLimitExceeded, batchv1.JobReasonDeadlineExceeded} { + t.Run(reason, func(t *testing.T) { + ctx := context.Background() + instance := &brokerv1alpha1.ClusterInstance{ + ObjectMeta: metav1.ObjectMeta{Name: recoveryInstanceName, Namespace: testNamespace}, + Spec: brokerv1alpha1.ClusterInstanceSpec{ + Type: brokerv1alpha1.TopologyCRC, + Template: brokerv1alpha1.ClusterTemplate{ + PullSecretRef: corev1.LocalObjectReference{Name: "pull-secret"}, + BundleSSHKeyRef: &corev1.LocalObjectReference{Name: "bundle-ssh-key"}, + Memory: "16Gi", + Cores: 4, + }, + }, + } + vmi := &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{Name: instance.Name, Namespace: instance.Namespace, UID: types.UID(recoveryVMIUID)}, + Status: kubevirtv1.VirtualMachineInstanceStatus{ + Phase: kubevirtv1.Running, + Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{{IP: "192.0.2.1"}}, + }, + } + vm := &kubevirtv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{Name: resources.VMName(instance.Name), Namespace: instance.Namespace}, + Status: kubevirtv1.VirtualMachineStatus{Ready: true}, + } + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Name: resources.CRCAgentJobName(instance.Name, recoveryVMIUID), Namespace: instance.Namespace}, + Status: batchv1.JobStatus{Conditions: []batchv1.JobCondition{{ + Type: batchv1.JobFailed, Status: corev1.ConditionTrue, Reason: reason, + }}}, + } + pullSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "pull-secret", Namespace: instance.Namespace}, Data: map[string][]byte{resources.PullSecretDataKey: []byte("pull")}} + sshSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "bundle-ssh-key", Namespace: instance.Namespace}, Data: map[string][]byte{"id_rsa": []byte("key")}} + ingress := &configv1.Ingress{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, Spec: configv1.IngressSpec{Domain: "apps.example.test"}} + c := newCRCRecoveryFakeClient(t, instance, vm, vmi, job, pullSecret, sshSecret, ingress) + r := &ClusterInstanceReconciler{Client: c, Scheme: c.Scheme()} + + if _, err := r.reconcileCRC(ctx, instance); err == nil { + t.Fatal("expected crc-agent Job failure") + } + + 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.PhaseFailed { + t.Fatalf("phase = %s, want Failed", got.Status.Phase) + } + condition := apimeta.FindStatusCondition(got.Status.Conditions, conditionTypeReady) + if condition == nil || condition.Status != metav1.ConditionFalse || condition.Reason != "ReconcileError" { + t.Fatalf("expected Ready=False with ReconcileError, got %+v", condition) + } + wantMessage := fmt.Sprintf("crc-agent Job %s failed: %s", job.Name, reason) + if condition.Message != wantMessage { + t.Fatalf("condition message = %q", condition.Message) + } + }) + } +} + func newCRCRecoveryFakeClient(t *testing.T, objects ...client.Object) client.Client { t.Helper() s := runtime.NewScheme() @@ -436,6 +499,9 @@ func newCRCRecoveryFakeClient(t *testing.T, objects ...client.Object) client.Cli if err := routev1.AddToScheme(s); err != nil { t.Fatalf("adding OpenShift Route scheme: %v", err) } + if err := configv1.AddToScheme(s); err != nil { + t.Fatalf("adding OpenShift Config scheme: %v", err) + } return fake.NewClientBuilder(). WithScheme(s). WithStatusSubresource(&brokerv1alpha1.ClusterInstance{}).