diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index a073018..ee9d214 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -37,3 +37,19 @@ jobs: - name: Run go linter run: make lint + + lint-operator: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: 1.24 + + - name: Run go linter + run: make -C deploy/operator lint diff --git a/deploy/operator/internal/controller/jumpstarter/compare.go b/deploy/operator/internal/controller/jumpstarter/compare.go index d22d287..547f1e4 100644 --- a/deploy/operator/internal/controller/jumpstarter/compare.go +++ b/deploy/operator/internal/controller/jumpstarter/compare.go @@ -19,7 +19,6 @@ package jumpstarter import ( "fmt" - "github.com/go-logr/logr" "github.com/pmezard/go-difflib/difflib" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -45,7 +44,7 @@ func deploymentNeedsUpdate(existing, desired *appsv1.Deployment) bool { } // configMapNeedsUpdate checks if a configmap needs to be updated using K8s semantic equality. -func configMapNeedsUpdate(existing, desired *corev1.ConfigMap, log logr.Logger) bool { +func configMapNeedsUpdate(existing, desired *corev1.ConfigMap) bool { // Compare labels (only if desired.Labels is non-nil) if desired.Labels != nil && !equality.Semantic.DeepEqual(existing.Labels, desired.Labels) { return true diff --git a/deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go b/deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go index 44f7403..114af83 100644 --- a/deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go +++ b/deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go @@ -139,15 +139,39 @@ func (r *Reconciler) ReconcileControllerEndpoint(ctx context.Context, owner meta "app": "jumpstarter-controller", } - // Create a service for each enabled service type - // This allows multiple service types to coexist for the same endpoint - // Note: ClusterIP uses no suffix (most common for in-cluster communication) - // LoadBalancer uses "-lb" suffix, NodePort uses "-np" suffix + // Create ingress and route resources + if err := r.createIngressAndRouteForController(ctx, owner, endpoint, servicePort, baseLabels); err != nil { + return err + } + + // Create LoadBalancer service + if err := r.createLoadBalancerServiceForController(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + + // Create NodePort service + if err := r.createNodePortServiceForController(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + + // Create ClusterIP service + if err := r.createClusterIPServiceForController(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + + // Create default service if no service type is enabled + if err := r.createDefaultServiceForController(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + return nil +} + +// createIngressAndRouteForController creates ingress and route resources for controller endpoint +func (r *Reconciler) createIngressAndRouteForController(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, baseLabels map[string]string) error { // Ingress resource (uses ClusterIP service) if endpoint.Ingress != nil && endpoint.Ingress.Enabled { serviceName := servicePort.Name - // Create the Ingress resource pointing to the ClusterIP service if err := r.createIngressForEndpoint(ctx, owner, serviceName, servicePort.Port, endpoint, baseLabels); err != nil { return err } @@ -156,29 +180,34 @@ func (r *Reconciler) ReconcileControllerEndpoint(ctx context.Context, owner meta // Route resource (uses ClusterIP service) if endpoint.Route != nil && endpoint.Route.Enabled { serviceName := servicePort.Name - // Create the Route resource pointing to the ClusterIP service if err := r.createRouteForEndpoint(ctx, owner, serviceName, servicePort.Port, endpoint, baseLabels); err != nil { return err } } - // LoadBalancer service + return nil +} + +// createLoadBalancerServiceForController creates LoadBalancer service for controller endpoint +func (r *Reconciler) createLoadBalancerServiceForController(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { if endpoint.LoadBalancer != nil && endpoint.LoadBalancer.Enabled { - if err := r.createService(ctx, owner, servicePort, "-lb", corev1.ServiceTypeLoadBalancer, - podSelector, baseLabels, endpoint.LoadBalancer.Annotations, endpoint.LoadBalancer.Labels); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "-lb", corev1.ServiceTypeLoadBalancer, + podSelector, baseLabels, endpoint.LoadBalancer.Annotations, endpoint.LoadBalancer.Labels) } + return nil +} - // NodePort service +// createNodePortServiceForController creates NodePort service for controller endpoint +func (r *Reconciler) createNodePortServiceForController(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { if endpoint.NodePort != nil && endpoint.NodePort.Enabled { - if err := r.createService(ctx, owner, servicePort, "-np", corev1.ServiceTypeNodePort, - podSelector, baseLabels, endpoint.NodePort.Annotations, endpoint.NodePort.Labels); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "-np", corev1.ServiceTypeNodePort, + podSelector, baseLabels, endpoint.NodePort.Annotations, endpoint.NodePort.Labels) } + return nil +} - // ClusterIP service (no suffix for cleaner in-cluster service names) +// createClusterIPServiceForController creates ClusterIP service for controller endpoint +func (r *Reconciler) createClusterIPServiceForController(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { // Create ClusterIP if explicitly enabled OR if Ingress/Route need it if (endpoint.ClusterIP != nil && endpoint.ClusterIP.Enabled) || (endpoint.Ingress != nil && endpoint.Ingress.Enabled) || @@ -189,12 +218,14 @@ func (r *Reconciler) ReconcileControllerEndpoint(ctx context.Context, owner meta annotations = endpoint.ClusterIP.Annotations labels = endpoint.ClusterIP.Labels } - if err := r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, - podSelector, baseLabels, annotations, labels); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, + podSelector, baseLabels, annotations, labels) } + return nil +} +// createDefaultServiceForController creates default ClusterIP service if no service type is enabled +func (r *Reconciler) createDefaultServiceForController(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { // If no service type is explicitly enabled, create a default ClusterIP service if (endpoint.LoadBalancer == nil || !endpoint.LoadBalancer.Enabled) && (endpoint.NodePort == nil || !endpoint.NodePort.Enabled) && @@ -203,12 +234,9 @@ func (r *Reconciler) ReconcileControllerEndpoint(ctx context.Context, owner meta (endpoint.Route == nil || !endpoint.Route.Enabled) { // TODO: Default to Route or Ingress depending of the type of cluster - if err := r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, - podSelector, baseLabels, nil, nil); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, + podSelector, baseLabels, nil, nil) } - return nil } @@ -231,15 +259,39 @@ func (r *Reconciler) ReconcileRouterReplicaEndpoint(ctx context.Context, owner m "app": baseAppLabel, // e.g., "jumpstarter-router-0" } - // Create a service for each enabled service type - // This allows multiple service types to coexist for the same endpoint - // Note: ClusterIP uses no suffix (most common for in-cluster communication) - // LoadBalancer uses "-lb" suffix, NodePort uses "-np" suffix + // Create ingress and route resources + if err := r.createIngressAndRouteForRouter(ctx, owner, endpoint, servicePort, baseLabels); err != nil { + return err + } + + // Create LoadBalancer service + if err := r.createLoadBalancerServiceForRouter(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + + // Create NodePort service + if err := r.createNodePortServiceForRouter(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + + // Create ClusterIP service + if err := r.createClusterIPServiceForRouter(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + // Create default service if no service type is enabled + if err := r.createDefaultServiceForRouter(ctx, owner, endpoint, servicePort, podSelector, baseLabels); err != nil { + return err + } + + return nil +} + +// createIngressAndRouteForRouter creates ingress and route resources for router endpoint +func (r *Reconciler) createIngressAndRouteForRouter(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, baseLabels map[string]string) error { // Ingress resource (uses ClusterIP service) if endpoint.Ingress != nil && endpoint.Ingress.Enabled { serviceName := servicePort.Name - // Create the Ingress resource pointing to the ClusterIP service if err := r.createIngressForEndpoint(ctx, owner, serviceName, servicePort.Port, endpoint, baseLabels); err != nil { return err } @@ -248,29 +300,34 @@ func (r *Reconciler) ReconcileRouterReplicaEndpoint(ctx context.Context, owner m // Route resource (uses ClusterIP service) if endpoint.Route != nil && endpoint.Route.Enabled { serviceName := servicePort.Name - // Create the Route resource pointing to the ClusterIP service if err := r.createRouteForEndpoint(ctx, owner, serviceName, servicePort.Port, endpoint, baseLabels); err != nil { return err } } - // LoadBalancer service + return nil +} + +// createLoadBalancerServiceForRouter creates LoadBalancer service for router endpoint +func (r *Reconciler) createLoadBalancerServiceForRouter(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { if endpoint.LoadBalancer != nil && endpoint.LoadBalancer.Enabled { - if err := r.createService(ctx, owner, servicePort, "-lb", corev1.ServiceTypeLoadBalancer, - podSelector, baseLabels, endpoint.LoadBalancer.Annotations, endpoint.LoadBalancer.Labels); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "-lb", corev1.ServiceTypeLoadBalancer, + podSelector, baseLabels, endpoint.LoadBalancer.Annotations, endpoint.LoadBalancer.Labels) } + return nil +} - // NodePort service +// createNodePortServiceForRouter creates NodePort service for router endpoint +func (r *Reconciler) createNodePortServiceForRouter(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { if endpoint.NodePort != nil && endpoint.NodePort.Enabled { - if err := r.createService(ctx, owner, servicePort, "-np", corev1.ServiceTypeNodePort, - podSelector, baseLabels, endpoint.NodePort.Annotations, endpoint.NodePort.Labels); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "-np", corev1.ServiceTypeNodePort, + podSelector, baseLabels, endpoint.NodePort.Annotations, endpoint.NodePort.Labels) } + return nil +} - // ClusterIP service (no suffix for cleaner in-cluster service names) +// createClusterIPServiceForRouter creates ClusterIP service for router endpoint +func (r *Reconciler) createClusterIPServiceForRouter(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { // Create ClusterIP if explicitly enabled OR if Ingress/Route need it if (endpoint.ClusterIP != nil && endpoint.ClusterIP.Enabled) || (endpoint.Ingress != nil && endpoint.Ingress.Enabled) || @@ -281,26 +338,23 @@ func (r *Reconciler) ReconcileRouterReplicaEndpoint(ctx context.Context, owner m annotations = endpoint.ClusterIP.Annotations labels = endpoint.ClusterIP.Labels } - if err := r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, - podSelector, baseLabels, annotations, labels); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, + podSelector, baseLabels, annotations, labels) } + return nil +} +// createDefaultServiceForRouter creates default ClusterIP service if no service type is enabled +func (r *Reconciler) createDefaultServiceForRouter(ctx context.Context, owner metav1.Object, endpoint *operatorv1alpha1.Endpoint, servicePort corev1.ServicePort, podSelector map[string]string, baseLabels map[string]string) error { // If no service type is explicitly enabled, create a default ClusterIP service if (endpoint.LoadBalancer == nil || !endpoint.LoadBalancer.Enabled) && (endpoint.NodePort == nil || !endpoint.NodePort.Enabled) && (endpoint.ClusterIP == nil || !endpoint.ClusterIP.Enabled) && (endpoint.Ingress == nil || !endpoint.Ingress.Enabled) && (endpoint.Route == nil || !endpoint.Route.Enabled) { - if err := r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, - podSelector, baseLabels, nil, nil); err != nil { - return err - } + return r.createService(ctx, owner, servicePort, "", corev1.ServiceTypeClusterIP, + podSelector, baseLabels, nil, nil) } - - // Note: Ingress resources are now created above. Route resources still need to be implemented. - return nil } diff --git a/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go b/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go index 1cb25de..f3c023d 100644 --- a/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go +++ b/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go @@ -422,7 +422,7 @@ func (r *JumpstarterReconciler) reconcileConfigMaps(ctx context.Context, jumpsta } // ConfigMap exists, check if update is needed - if !configMapNeedsUpdate(existingConfigMap, desiredConfigMap, log) { + if !configMapNeedsUpdate(existingConfigMap, desiredConfigMap) { log.V(1).Info("ConfigMap is up to date, skipping update", "name", existingConfigMap.Name, "namespace", existingConfigMap.Namespace) diff --git a/deploy/operator/test/e2e/e2e_test.go b/deploy/operator/test/e2e/e2e_test.go index cce798b..530c0fe 100644 --- a/deploy/operator/test/e2e/e2e_test.go +++ b/deploy/operator/test/e2e/e2e_test.go @@ -29,6 +29,7 @@ import ( appsv1 "k8s.io/api/apps/v1" authenticationv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -107,7 +108,7 @@ var _ = Describe("Manager", Ordered, func() { req := clientset.CoreV1().Pods(namespace).GetLogs(controllerPodName, &corev1.PodLogOptions{}) podLogs, err := req.Stream(ctx) if err == nil { - defer podLogs.Close() + defer podLogs.Close() //nolint:errcheck // Close errors on log streams are not actionable in test cleanup buf := new(bytes.Buffer) _, _ = io.Copy(buf, podLogs) _, _ = fmt.Fprintf(GinkgoWriter, "Controller logs:\n %s", buf.String()) @@ -122,7 +123,7 @@ var _ = Describe("Manager", Ordered, func() { _, _ = fmt.Fprintf(GinkgoWriter, "Kubernetes events:\n") for _, event := range eventList.Items { _, _ = fmt.Fprintf(GinkgoWriter, "%s %s %s %s\n", - event.LastTimestamp.Time.Format(time.RFC3339), + event.LastTimestamp.Format(time.RFC3339), event.InvolvedObject.Name, event.Reason, event.Message) @@ -135,7 +136,7 @@ var _ = Describe("Manager", Ordered, func() { req = clientset.CoreV1().Pods(namespace).GetLogs("curl-metrics", &corev1.PodLogOptions{}) metricsLogs, err := req.Stream(ctx) if err == nil { - defer metricsLogs.Close() + defer metricsLogs.Close() //nolint:errcheck // Close errors on log streams are not actionable in test cleanup buf := new(bytes.Buffer) _, _ = io.Copy(buf, metricsLogs) _, _ = fmt.Fprintf(GinkgoWriter, "Metrics logs:\n %s", buf.String()) @@ -239,27 +240,28 @@ var _ = Describe("Manager", Ordered, func() { Expect(err).NotTo(HaveOccurred(), "Metrics service should exist") By("getting the service account token") - token, err := serviceAccountToken() - Expect(err).NotTo(HaveOccurred()) + token := serviceAccountToken() Expect(token).NotTo(BeEmpty()) By("waiting for the metrics endpoint to be ready") verifyMetricsEndpointReady := func(g Gomega) { - endpoints := &corev1.Endpoints{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Name: metricsServiceName, - Namespace: namespace, - }, endpoints) + endpointSliceList := &discoveryv1.EndpointSliceList{} + err := k8sClient.List(ctx, endpointSliceList, client.InNamespace(namespace), client.MatchingLabels{ + "kubernetes.io/service-name": metricsServiceName, + }) g.Expect(err).NotTo(HaveOccurred()) hasPort := false - for _, subset := range endpoints.Subsets { - for _, port := range subset.Ports { - if port.Port == 8443 { + for _, endpointSlice := range endpointSliceList.Items { + for _, port := range endpointSlice.Ports { + if port.Port != nil && *port.Port == 8443 { hasPort = true break } } + if hasPort { + break + } } g.Expect(hasPort).To(BeTrue(), "Metrics endpoint is not ready") } @@ -270,7 +272,7 @@ var _ = Describe("Manager", Ordered, func() { req := clientset.CoreV1().Pods(namespace).GetLogs(controllerPodName, &corev1.PodLogOptions{}) podLogs, err := req.Stream(ctx) g.Expect(err).NotTo(HaveOccurred()) - defer podLogs.Close() + defer podLogs.Close() //nolint:errcheck // Close errors on log streams are not actionable in test cleanup buf := new(bytes.Buffer) _, _ = io.Copy(buf, podLogs) g.Expect(buf.String()).To(ContainSubstring("controller-runtime.metrics\tServing metrics server"), @@ -689,7 +691,7 @@ provisioning: // serviceAccountToken returns a token for the specified service account in the given namespace. // It uses the Kubernetes TokenRequest API to generate a token by directly calling the API. -func serviceAccountToken() (string, error) { +func serviceAccountToken() string { var token string verifyTokenCreation := func(g Gomega) { // Create a token request for the service account @@ -713,7 +715,7 @@ func serviceAccountToken() (string, error) { } Eventually(verifyTokenCreation).Should(Succeed()) - return token, nil + return token } // getMetricsOutput retrieves and returns the logs from the curl pod used to access the metrics endpoint. @@ -722,7 +724,7 @@ func getMetricsOutput() string { req := clientset.CoreV1().Pods(namespace).GetLogs("curl-metrics", &corev1.PodLogOptions{}) podLogs, err := req.Stream(ctx) Expect(err).NotTo(HaveOccurred(), "Failed to retrieve logs from curl pod") - defer podLogs.Close() + defer podLogs.Close() //nolint:errcheck // Close errors on log streams are not actionable in test cleanup buf := new(bytes.Buffer) _, _ = io.Copy(buf, podLogs) diff --git a/deploy/operator/test/e2e/utils.go b/deploy/operator/test/e2e/utils.go index d68ca2f..3c01a1c 100644 --- a/deploy/operator/test/e2e/utils.go +++ b/deploy/operator/test/e2e/utils.go @@ -22,8 +22,8 @@ import ( "time" "github.com/google/uuid" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + . "github.com/onsi/ginkgo/v2" //nolint:staticcheck // Dot import used for convenience in test utilities + . "github.com/onsi/gomega" //nolint:staticcheck // Dot import used for convenience in test utilities corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"