diff --git a/.github/workflows/govulncheck.yml b/.github/workflows/govulncheck.yml index 839fbd6d..90ce86ef 100644 --- a/.github/workflows/govulncheck.yml +++ b/.github/workflows/govulncheck.yml @@ -22,7 +22,7 @@ jobs: timeout-minutes: 15 steps: - name: Clone the code - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 5c1c554d..bce72dac 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Clone the code - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index 79042181..d3779cba 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Clone the code - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6c0baadc..426f4482 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Clone the code - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index d096eebf..903bf77e 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Clone the code - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 diff --git a/Dockerfile b/Dockerfile index 52e808c3..cbb850f8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ # https://github.com/kubernetes-sigs/kubebuilder/blob/v4.11.1/pkg/plugins/golang/v4/scaffolds/internal/templates/dockerfile.go # Build the manager binary -FROM golang:1.26.3 AS builder +FROM golang:1.26.4 AS builder ARG TARGETOS ARG TARGETARCH diff --git a/Dockerfile.ci b/Dockerfile.ci index de3bb75f..3c655a65 100644 --- a/Dockerfile.ci +++ b/Dockerfile.ci @@ -1,5 +1,5 @@ -ARG BUILDER_IMAGE=registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.26-openshift-5.0 -ARG BASE_IMAGE=registry.ci.openshift.org/ocp/5.0:base-rhel9 +ARG BUILDER_IMAGE=registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.23 +ARG BASE_IMAGE=registry.ci.openshift.org/ocp/4.23:base-rhel9 FROM ${BUILDER_IMAGE} AS builder ARG TARGETOS diff --git a/internal/controller/mcpserver_controller.go b/internal/controller/mcpserver_controller.go index 54410016..aa09a82d 100644 --- a/internal/controller/mcpserver_controller.go +++ b/internal/controller/mcpserver_controller.go @@ -115,6 +115,8 @@ const ( eventActionMCPHandshakeRetriesExhausted = "MCPHandshakeRetriesExhausted" // eventActionDeploymentReconcileFailed is the reporting action when Deployment reconciliation fails. eventActionDeploymentReconcileFailed = "DeploymentReconcileFailed" + // eventActionServiceReconcileFailed is the reporting action when Service reconciliation fails. + eventActionServiceReconcileFailed = "ServiceReconcileFailed" // requeueDelayMCPHandshake is the initial delay before requeuing when an MCP handshake fails. requeueDelayMCPHandshake = 10 * time.Second @@ -295,6 +297,10 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( recordCondition(mcpServer.Name, mcpServer.Namespace, readyCondition.Type, string(readyCondition.Status), readyCondition.Reason) + if !duplicateServiceUnavailable(mcpServer.Status.Conditions, readyCondition.Message) { + r.emitServiceReconcileFailed(mcpServer, readyCondition.Message) + } + status := acv1alpha1.MCPServerStatus(). WithObservedGeneration(mcpServer.Generation). WithDeploymentName(existingDeployment.Name). @@ -518,6 +524,14 @@ func (r *MCPServerReconciler) emitDeploymentReconcileFailed(mcpServer *mcpv1alph "MCPServer %s: %s", mcpServer.Name, message) } +func (r *MCPServerReconciler) emitServiceReconcileFailed(mcpServer *mcpv1alpha1.MCPServer, message string) { + if r.Recorder == nil { + return + } + r.Recorder.Eventf(mcpServer, nil, corev1.EventTypeWarning, ReasonServiceUnavailable, eventActionServiceReconcileFailed, + "MCPServer %s: %s", mcpServer.Name, message) +} + func (r *MCPServerReconciler) emitMCPHandshakeFailed(mcpServer *mcpv1alpha1.MCPServer, message string) { if r.Recorder == nil { return diff --git a/internal/controller/mcpserver_controller_conditions.go b/internal/controller/mcpserver_controller_conditions.go index 18cf9482..467c8100 100644 --- a/internal/controller/mcpserver_controller_conditions.go +++ b/internal/controller/mcpserver_controller_conditions.go @@ -296,3 +296,9 @@ func duplicateDeploymentUnavailable(conditions []metav1.Condition, message strin return prevReady != nil && prevReady.Status == metav1.ConditionFalse && prevReady.Reason == ReasonDeploymentUnavailable && prevReady.Message == message } + +func duplicateServiceUnavailable(conditions []metav1.Condition, message string) bool { + prevReady := meta.FindStatusCondition(conditions, ConditionTypeReady) + return prevReady != nil && prevReady.Status == metav1.ConditionFalse && + prevReady.Reason == ReasonServiceUnavailable && prevReady.Message == message +} diff --git a/internal/controller/mcpserver_controller_conditions_test.go b/internal/controller/mcpserver_controller_conditions_test.go index 19381057..ba49ba80 100644 --- a/internal/controller/mcpserver_controller_conditions_test.go +++ b/internal/controller/mcpserver_controller_conditions_test.go @@ -701,4 +701,18 @@ var _ = Describe("status condition helpers", func() { {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Reason: ReasonDeploymentUnavailable, Message: msg}, }, msg)).To(BeTrue()) }) + + It("duplicateServiceUnavailable returns true only for matching Ready=False ServiceUnavailable message", func() { + msg := "Failed to reconcile Service: simulated failure" + Expect(duplicateServiceUnavailable(nil, msg)).To(BeFalse()) + Expect(duplicateServiceUnavailable([]metav1.Condition{ + {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Reason: ReasonDeploymentUnavailable, Message: msg}, + }, msg)).To(BeFalse()) + Expect(duplicateServiceUnavailable([]metav1.Condition{ + {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Reason: ReasonServiceUnavailable, Message: "other"}, + }, msg)).To(BeFalse()) + Expect(duplicateServiceUnavailable([]metav1.Condition{ + {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Reason: ReasonServiceUnavailable, Message: msg}, + }, msg)).To(BeTrue()) + }) }) diff --git a/internal/controller/mcpserver_controller_service_test.go b/internal/controller/mcpserver_controller_service_test.go index 5953b1ce..8e262296 100644 --- a/internal/controller/mcpserver_controller_service_test.go +++ b/internal/controller/mcpserver_controller_service_test.go @@ -19,6 +19,8 @@ package controller import ( "context" "fmt" + "strings" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -581,6 +583,100 @@ var _ = Describe("MCPServer Controller - Service Reconciliation Failures", func( }) }) +var _ = Describe("MCPServer Controller - Service Reconcile Events", func() { + const resourceName = "test-service-events" + + ctx := context.Background() + + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", + } + + BeforeEach(func() { + resource := newTestMCPServer(resourceName) + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + + AfterEach(func() { + resource := &mcpv1alpha1.MCPServer{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + if err == nil { + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + } + }) + + It("should emit a Warning ServiceReconcileFailed event only when service error message changes", func() { + failMsg := "simulated service creation failure" + reconciler, fr := newReconcilerForTestWithFakeEvents(k8sClient, k8sClient.Scheme()) + + wrappedClient, err := client.NewWithWatch(cfg, client.Options{Scheme: k8sClient.Scheme()}) + Expect(err).NotTo(HaveOccurred()) + + interceptedClient := interceptor.NewClient(wrappedClient, interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if _, ok := obj.(*corev1.Service); ok { + return fmt.Errorf("%s", failMsg) + } + return c.Create(ctx, obj, opts...) + }, + }) + reconciler.Client = interceptedClient + + By("First service reconcile failure — Warning event emitted once") + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).To(HaveOccurred()) + + var serviceFailedEvent string + Eventually(func(g Gomega) { + for _, ev := range drainEvents(fr.Events) { + if strings.Contains(ev, corev1.EventTypeWarning) && strings.Contains(ev, ReasonServiceUnavailable) { + serviceFailedEvent = ev + break + } + } + g.Expect(serviceFailedEvent).NotTo(BeEmpty()) + g.Expect(serviceFailedEvent).To(ContainSubstring(resourceName)) + g.Expect(serviceFailedEvent).To(ContainSubstring("Failed to reconcile Service")) + g.Expect(serviceFailedEvent).To(ContainSubstring(failMsg)) + }).Should(Succeed()) + + By("Second reconcile with same error — no duplicate service failed event") + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).To(HaveOccurred()) + Consistently(fr.Events, 300*time.Millisecond, 20*time.Millisecond).ShouldNot(Receive()) + + By("Change error message — second Warning event emitted") + failMsg = "simulated service ownership failure" + interceptedClient = interceptor.NewClient(wrappedClient, interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if _, ok := obj.(*corev1.Service); ok { + return fmt.Errorf("%s", failMsg) + } + return c.Create(ctx, obj, opts...) + }, + }) + reconciler.Client = interceptedClient + + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).To(HaveOccurred()) + + var secondServiceFailedEvent string + Eventually(func(g Gomega) { + for _, ev := range drainEvents(fr.Events) { + if strings.Contains(ev, corev1.EventTypeWarning) && strings.Contains(ev, ReasonServiceUnavailable) { + secondServiceFailedEvent = ev + break + } + } + g.Expect(secondServiceFailedEvent).NotTo(BeEmpty()) + g.Expect(secondServiceFailedEvent).To(ContainSubstring(resourceName)) + g.Expect(secondServiceFailedEvent).To(ContainSubstring(failMsg)) + g.Expect(secondServiceFailedEvent).NotTo(Equal(serviceFailedEvent)) + }).Should(Succeed()) + }) +}) + var _ = Describe("MCPServer Controller - Server-Side Apply for Status", func() { const resourceName = "test-ssa-status" const subResourceStatus = "status"