From 2bbb2e41037466eb3dadc199db1c6d0d206bc6e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wilson=20J=C3=BAnior?= Date: Wed, 17 Jan 2024 17:11:39 -0300 Subject: [PATCH 1/4] Add addresses fields to status Co-authored-by: Claudio Gisch --- api/v1alpha1/nginx_types.go | 10 +++- config/crd/bases/nginx.tsuru.io_nginxes.yaml | 22 ++++++++ controllers/nginx_controller.go | 54 ++++++++++++++++++-- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/nginx_types.go b/api/v1alpha1/nginx_types.go index b5270a19..55329699 100644 --- a/api/v1alpha1/nginx_types.go +++ b/api/v1alpha1/nginx_types.go @@ -17,6 +17,8 @@ import ( // +kubebuilder:printcolumn:name="Current",type=integer,JSONPath=`.status.currentReplicas` // +kubebuilder:printcolumn:name="Desired",type=integer,JSONPath=`.spec.replicas` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` +// +kubebuilder:printcolumn:name="Ingress IPs",type=string,JSONPath=`.status.ingresses[*].ips[*]` +// +kubebuilder:printcolumn:name="Service IPs",type=string,JSONPath=`.status.services[*].ips[*]` // Nginx is the Schema for the nginxes API type Nginx struct { @@ -262,12 +264,16 @@ type DeploymentStatus struct { type ServiceStatus struct { // Name is the name of the Service created by nginx - Name string `json:"name"` + Name string `json:"name"` + IPs []string `json:"ips,omitempty"` + Hostnames []string `json:"hostnames,omitempty"` } type IngressStatus struct { // Name is the name of the Ingress created by nginx - Name string `json:"name"` + Name string `json:"name"` + IPs []string `json:"ips,omitempty"` + Hostnames []string `json:"hostnames,omitempty"` } func init() { diff --git a/config/crd/bases/nginx.tsuru.io_nginxes.yaml b/config/crd/bases/nginx.tsuru.io_nginxes.yaml index b5da1443..f801636d 100644 --- a/config/crd/bases/nginx.tsuru.io_nginxes.yaml +++ b/config/crd/bases/nginx.tsuru.io_nginxes.yaml @@ -26,6 +26,12 @@ spec: - jsonPath: .metadata.creationTimestamp name: Age type: date + - jsonPath: .status.ingresses[*].ips[*] + name: Ingress IPs + type: string + - jsonPath: .status.services[*].ips[*] + name: Service IPs + type: string name: v1alpha1 schema: openAPIV3Schema: @@ -5680,6 +5686,14 @@ spec: ingresses: items: properties: + hostnames: + items: + type: string + type: array + ips: + items: + type: string + type: array name: description: Name is the name of the Ingress created by nginx type: string @@ -5693,6 +5707,14 @@ spec: services: items: properties: + hostnames: + items: + type: string + type: array + ips: + items: + type: string + type: array name: description: Name is the name of the Service created by nginx type: string diff --git a/controllers/nginx_controller.go b/controllers/nginx_controller.go index 5fac9c9c..be5e41b6 100644 --- a/controllers/nginx_controller.go +++ b/controllers/nginx_controller.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "reflect" + "slices" "sort" "strings" "time" @@ -51,6 +52,8 @@ func (r *NginxReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&nginxv1alpha1.Nginx{}). Owns(&appsv1.Deployment{}). + Owns(&networkingv1.Ingress{}). + Owns(&corev1.Service{}). Complete(r) } @@ -239,6 +242,12 @@ func (r *NginxReconciler) reconcileIngress(ctx context.Context, nginx *nginxv1al return nil } + for key, value := range currentIngress.Annotations { + if newIngress.Annotations[key] == "" { + newIngress.Annotations[key] = value + } + } + newIngress.ResourceVersion = currentIngress.ResourceVersion newIngress.Finalizers = currentIngress.Finalizers @@ -250,8 +259,13 @@ func shouldUpdateIngress(currentIngress, newIngress *networkingv1.Ingress) bool return false } - return !reflect.DeepEqual(currentIngress.Annotations, newIngress.Annotations) || - !reflect.DeepEqual(currentIngress.Labels, newIngress.Labels) || + for key, value := range newIngress.Annotations { + if currentIngress.Annotations[key] != value { + return true + } + } + + return !reflect.DeepEqual(currentIngress.Labels, newIngress.Labels) || !reflect.DeepEqual(currentIngress.Spec, newIngress.Spec) } @@ -361,9 +375,24 @@ func listServices(ctx context.Context, c client.Client, nginx *nginxv1alpha1.Ngi var services []nginxv1alpha1.ServiceStatus for _, s := range serviceList.Items { - services = append(services, nginxv1alpha1.ServiceStatus{ + svc := nginxv1alpha1.ServiceStatus{ Name: s.Name, - }) + } + + for _, ingStatus := range s.Status.LoadBalancer.Ingress { + if ingStatus.IP != "" { + svc.IPs = append(svc.IPs, ingStatus.IP) + } + + if ingStatus.Hostname != "" { + svc.Hostnames = append(svc.Hostnames, ingStatus.Hostname) + } + } + + slices.Sort(svc.IPs) + slices.Sort(svc.Hostnames) + + services = append(services, svc) } sort.Slice(services, func(i, j int) bool { @@ -386,7 +415,22 @@ func listIngresses(ctx context.Context, c client.Client, nginx *nginxv1alpha1.Ng var ingresses []nginxv1alpha1.IngressStatus for _, i := range ingressList.Items { - ingresses = append(ingresses, nginxv1alpha1.IngressStatus{Name: i.Name}) + ing := nginxv1alpha1.IngressStatus{Name: i.Name} + + for _, ingStatus := range i.Status.LoadBalancer.Ingress { + if ingStatus.IP != "" { + ing.IPs = append(ing.IPs, ingStatus.IP) + } + + if ingStatus.Hostname != "" { + ing.Hostnames = append(ing.Hostnames, ingStatus.Hostname) + } + } + + slices.Sort(ing.IPs) + slices.Sort(ing.Hostnames) + + ingresses = append(ingresses, ing) } sort.Slice(ingresses, func(i, j int) bool { From ddcff05becef95fa63c449947f2343794eb7896f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wilson=20J=C3=BAnior?= Date: Thu, 18 Jan 2024 10:29:29 -0300 Subject: [PATCH 2/4] Increase golang version --- .github/workflows/ci.yaml | 8 ++++---- Dockerfile | 2 +- go.mod | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f63c166e..297268e7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,7 +9,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v4 with: - go-version: '1.20' + go-version: '1.21' - run: make test lint: @@ -18,10 +18,10 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v4 with: - go-version: '1.20' + go-version: '1.21' - uses: golangci/golangci-lint-action@v3 with: - version: v1.53 + version: v1.55 integration: runs-on: ubuntu-latest @@ -36,7 +36,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v4 with: - go-version: '1.20' + go-version: '1.21' - name: Setup run: | kubectl create namespace nginx-operator-system diff --git a/Dockerfile b/Dockerfile index 10a79b41..928f0e9a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.20 as builder +FROM golang:1.21 as builder WORKDIR /workspace # Copy the Go Modules and Go source code COPY ./ ./ diff --git a/go.mod b/go.mod index 7cf07dc3..f706c2a9 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/tsuru/nginx-operator -go 1.20 +go 1.21 require ( github.com/go-logr/logr v1.2.0 From 01411af3a75b7c6fb44c313f7d9c50f3ef77aefb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wilson=20J=C3=BAnior?= Date: Thu, 18 Jan 2024 13:23:37 -0300 Subject: [PATCH 3/4] Add tests --- controllers/nginx_controller_test.go | 354 ++++++++++++++++++++++++++- 1 file changed, 343 insertions(+), 11 deletions(-) diff --git a/controllers/nginx_controller_test.go b/controllers/nginx_controller_test.go index 3d0dae32..38443bb4 100644 --- a/controllers/nginx_controller_test.go +++ b/controllers/nginx_controller_test.go @@ -7,6 +7,7 @@ package controllers import ( "context" "encoding/json" + "fmt" "sync" "testing" @@ -24,12 +25,12 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/tsuru/nginx-operator/api/v1alpha1" - nginxv1alpha1 "github.com/tsuru/nginx-operator/api/v1alpha1" ) func TestNginxReconciler_reconcileDeployment(t *testing.T) { @@ -88,7 +89,7 @@ func TestNginxReconciler_reconcileDeployment(t *testing.T) { expected := v1alpha1.NginxSpec{ Image: "nginx:alpine", Replicas: func(n int32) *int32 { return &n }(int32(2)), - PodTemplate: nginxv1alpha1.NginxPodTemplateSpec{ + PodTemplate: v1alpha1.NginxPodTemplateSpec{ Ports: []corev1.ContainerPort{ {Name: "http", ContainerPort: int32(8080), Protocol: corev1.ProtocolTCP}, {Name: "https", ContainerPort: int32(8443), Protocol: corev1.ProtocolTCP}, @@ -127,7 +128,7 @@ func TestNginxReconciler_reconcileDeployment(t *testing.T) { expected := v1alpha1.NginxSpec{ Image: "nginx:1.22.0", - PodTemplate: nginxv1alpha1.NginxPodTemplateSpec{ + PodTemplate: v1alpha1.NginxPodTemplateSpec{ Ports: []corev1.ContainerPort{ {Name: "http", ContainerPort: int32(8080), Protocol: corev1.ProtocolTCP}, {Name: "https", ContainerPort: int32(8443), Protocol: corev1.ProtocolTCP}, @@ -663,7 +664,7 @@ func TestNginxReconciler_reconcileIngress(t *testing.T) { } tests := map[string]struct { - nginx *nginxv1alpha1.Nginx + nginx *v1alpha1.Nginx expectedError string assert func(t *testing.T, c client.Client, nginx *v1alpha1.Nginx) }{ @@ -672,14 +673,14 @@ func TestNginxReconciler_reconcileIngress(t *testing.T) { }, "when ingress does not exist, should create one": { - nginx: &nginxv1alpha1.Nginx{ + nginx: &v1alpha1.Nginx{ ObjectMeta: metav1.ObjectMeta{ Name: "my-nginx-2", Namespace: "default", ResourceVersion: "666", }, - Spec: nginxv1alpha1.NginxSpec{ - Ingress: &nginxv1alpha1.NginxIngress{ + Spec: v1alpha1.NginxSpec{ + Ingress: &v1alpha1.NginxIngress{ IngressClassName: func(s string) *string { return &s }("custom-class"), }, }, @@ -805,15 +806,15 @@ func TestNginxReconciler_reconcileIngress(t *testing.T) { }, "when TLS is set, should not set the default backend": { - nginx: &nginxv1alpha1.Nginx{ + nginx: &v1alpha1.Nginx{ ObjectMeta: metav1.ObjectMeta{ Name: "my-nginx-2", Namespace: "default", ResourceVersion: "666", }, - Spec: nginxv1alpha1.NginxSpec{ - Ingress: &nginxv1alpha1.NginxIngress{}, - TLS: []nginxv1alpha1.NginxTLS{ + Spec: v1alpha1.NginxSpec{ + Ingress: &v1alpha1.NginxIngress{}, + TLS: []v1alpha1.NginxTLS{ {SecretName: "example-com-certs", Hosts: []string{"www.example.com"}}, }, }, @@ -1023,6 +1024,337 @@ func TestNginxReconciler_shouldManageNginx(t *testing.T) { } } +func TestShouldUpdateIngress(t *testing.T) { + tests := []struct { + current *networkingv1.Ingress + new *networkingv1.Ingress + expected bool + }{ + { + current: nil, + new: &networkingv1.Ingress{}, + expected: false, + }, + { + current: &networkingv1.Ingress{}, + new: nil, + expected: false, + }, + { + current: &networkingv1.Ingress{}, + new: &networkingv1.Ingress{}, + expected: false, + }, + { + current: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "a": "1", + "b": "2", + "c": "3", + }, + }, + }, + new: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "a": "1", + "d": "4", + }, + }, + }, + expected: true, + }, + { + current: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "a": "1", + "b": "2", + "c": "3", + }, + }, + }, + new: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "a": "1", + "b": "2", + }, + }, + }, + expected: false, + }, + { + current: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "1", + "b": "2", + "c": "3", + }, + }, + }, + new: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "1", + "b": "2", + }, + }, + }, + expected: true, + }, + { + current: &networkingv1.Ingress{ + Spec: networkingv1.IngressSpec{ + IngressClassName: pointer.String("old"), + }, + }, + new: &networkingv1.Ingress{ + Spec: networkingv1.IngressSpec{ + IngressClassName: pointer.String("new"), + }, + }, + expected: true, + }, + { + current: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "1", + "b": "2", + "c": "3", + }, + Annotations: map[string]string{ + "a": "1", + "b": "2", + "c": "3", + }, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: pointer.String("class"), + }, + }, + new: &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "1", + "b": "2", + "c": "3", + }, + Annotations: map[string]string{ + "a": "1", + "b": "2", + "c": "3", + }, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: pointer.String("class"), + }, + }, + expected: false, + }, + } + + for index, test := range tests { + t.Run(fmt.Sprintf("case %d", index), func(t *testing.T) { + update := shouldUpdateIngress(test.current, test.new) + assert.Equal(t, test.expected, update) + }) + } +} + +func TestListServices(t *testing.T) { + nginx := &v1alpha1.Nginx{ObjectMeta: metav1.ObjectMeta{Name: "my-nginx", Namespace: "default"}} + + resources := []runtime.Object{ + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nginx-service", + Namespace: "default", + Labels: map[string]string{ + "nginx.tsuru.io/app": "nginx", + "nginx.tsuru.io/resource-name": "my-nginx", + }, + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "1.1.1.2", + }, + { + IP: "1.1.1.1", + }, + }, + }, + }, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nginx-service-old", + Namespace: "default", + Labels: map[string]string{ + "nginx.tsuru.io/app": "nginx", + "nginx.tsuru.io/resource-name": "my-nginx", + }, + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "1.1.1.3", + }, + { + IP: "1.1.1.4", + }, + }, + }, + }, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nginx-service-hostname", + Namespace: "default", + Labels: map[string]string{ + "nginx.tsuru.io/app": "nginx", + "nginx.tsuru.io/resource-name": "my-nginx", + }, + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + Hostname: "hello02", + }, + { + Hostname: "hello01", + }, + }, + }, + }, + }, + } + + client := fake.NewClientBuilder(). + WithScheme(newScheme()). + WithRuntimeObjects(resources...). + Build() + + svcs, err := listServices(context.Background(), client, nginx) + assert.NoError(t, err) + assert.Equal(t, []v1alpha1.ServiceStatus{ + { + Name: "my-nginx-service", + IPs: []string{"1.1.1.1", "1.1.1.2"}, + }, + { + Name: "my-nginx-service-hostname", + Hostnames: []string{"hello01", "hello02"}, + }, + { + Name: "my-nginx-service-old", + IPs: []string{"1.1.1.3", "1.1.1.4"}, + }, + }, svcs) +} + +func TestListIngress(t *testing.T) { + nginx := &v1alpha1.Nginx{ObjectMeta: metav1.ObjectMeta{Name: "my-nginx", Namespace: "default"}} + + resources := []runtime.Object{ + &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nginx-ing", + Namespace: "default", + Labels: map[string]string{ + "nginx.tsuru.io/app": "nginx", + "nginx.tsuru.io/resource-name": "my-nginx", + }, + }, + Status: networkingv1.IngressStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "1.1.1.2", + }, + { + IP: "1.1.1.1", + }, + }, + }, + }, + }, + &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nginx-ing-old", + Namespace: "default", + Labels: map[string]string{ + "nginx.tsuru.io/app": "nginx", + "nginx.tsuru.io/resource-name": "my-nginx", + }, + }, + Status: networkingv1.IngressStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "1.1.1.3", + }, + { + IP: "1.1.1.4", + }, + }, + }, + }, + }, + &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nginx-ing-hostname", + Namespace: "default", + Labels: map[string]string{ + "nginx.tsuru.io/app": "nginx", + "nginx.tsuru.io/resource-name": "my-nginx", + }, + }, + Status: networkingv1.IngressStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + Hostname: "hello02", + }, + { + Hostname: "hello01", + }, + }, + }, + }, + }, + } + + client := fake.NewClientBuilder(). + WithScheme(newScheme()). + WithRuntimeObjects(resources...). + Build() + + svcs, err := listIngresses(context.Background(), client, nginx) + assert.NoError(t, err) + assert.Equal(t, []v1alpha1.IngressStatus{ + { + Name: "my-nginx-ing", + IPs: []string{"1.1.1.1", "1.1.1.2"}, + }, + { + Name: "my-nginx-ing-hostname", + Hostnames: []string{"hello01", "hello02"}, + }, + { + Name: "my-nginx-ing-old", + IPs: []string{"1.1.1.3", "1.1.1.4"}, + }, + }, svcs) +} + func newScheme() *runtime.Scheme { scheme := runtime.NewScheme() clientgoscheme.AddToScheme(scheme) From 41b371c45748b406d4307b173864fe297815ed35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wilson=20J=C3=BAnior?= Date: Thu, 18 Jan 2024 14:53:51 -0300 Subject: [PATCH 4/4] Test of ensure of ingress annotations to avoid loop of changes --- controllers/nginx_controller_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/controllers/nginx_controller_test.go b/controllers/nginx_controller_test.go index 38443bb4..f9bad2d7 100644 --- a/controllers/nginx_controller_test.go +++ b/controllers/nginx_controller_test.go @@ -619,6 +619,9 @@ func TestNginxReconciler_reconcileIngress(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "my-nginx-1", Namespace: "default", + Annotations: map[string]string{ + "cloudprovider.ingress/status": "this annotation created by ingress controller", + }, }, Spec: networkingv1.IngressSpec{ DefaultBackend: &networkingv1.IngressBackend{ @@ -766,7 +769,10 @@ func TestNginxReconciler_reconcileIngress(t *testing.T) { err := c.Get(context.TODO(), types.NamespacedName{Name: "my-nginx-1", Namespace: "default"}, &got) require.NoError(t, err) - assert.Equal(t, map[string]string{"custom.nginx.tsuru.io/foo": "bar"}, got.Annotations) + assert.Equal(t, map[string]string{ + "cloudprovider.ingress/status": "this annotation created by ingress controller", + "custom.nginx.tsuru.io/foo": "bar", + }, got.Annotations) assert.Equal(t, map[string]string{ "nginx.tsuru.io/app": "nginx", "nginx.tsuru.io/resource-name": "my-nginx-1",