From 1f1f4b6db5391f1df80813b54a5c653889d42d35 Mon Sep 17 00:00:00 2001 From: Inbaraj S Date: Tue, 3 Oct 2023 12:40:51 +0530 Subject: [PATCH] Added additional checks for lb exceptions --- .../templates/deployment.yaml | 6 +-- .../templates/rbac.yaml | 16 +++--- .../templates/service.yaml | 4 +- .../templates/serviceaccount.yaml | 4 +- .../templates/webhook.yaml | 4 +- helm/oci-native-ingress-controller/Chart.yaml | 4 +- .../oci-native-ingress-controller/values.yaml | 2 +- pkg/controllers/ingressclass/ingressclass.go | 15 ++++-- .../ingressclass/ingressclass_test.go | 51 +++++++++++++++++++ 9 files changed, 81 insertions(+), 25 deletions(-) diff --git a/deploy/manifests/oci-native-ingress-controller/templates/deployment.yaml b/deploy/manifests/oci-native-ingress-controller/templates/deployment.yaml index 6f41ab8c..bb2d3cee 100644 --- a/deploy/manifests/oci-native-ingress-controller/templates/deployment.yaml +++ b/deploy/manifests/oci-native-ingress-controller/templates/deployment.yaml @@ -18,10 +18,10 @@ metadata: name: oci-native-ingress-controller namespace: native-ingress-controller-system labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm spec: replicas: 1 @@ -51,7 +51,7 @@ spec: readOnlyRootFilesystem: true runAsNonRoot: true runAsUser: 1000 - image: "ghcr.io/oracle/oci-native-ingress-controller:v1.0.0" + image: "ghcr.io/oracle/oci-native-ingress-controller:v1.2.0" imagePullPolicy: Always args: - --lease-lock-name=oci-native-ingress-controller diff --git a/deploy/manifests/oci-native-ingress-controller/templates/rbac.yaml b/deploy/manifests/oci-native-ingress-controller/templates/rbac.yaml index 79e0096f..9026cc6f 100644 --- a/deploy/manifests/oci-native-ingress-controller/templates/rbac.yaml +++ b/deploy/manifests/oci-native-ingress-controller/templates/rbac.yaml @@ -11,10 +11,10 @@ kind: ClusterRole metadata: name: oci-native-ingress-controller-role labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm rules: - apiGroups: [""] @@ -48,10 +48,10 @@ kind: ClusterRoleBinding metadata: name: oci-native-ingress-controller-rolebinding labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm roleRef: apiGroup: rbac.authorization.k8s.io @@ -69,10 +69,10 @@ metadata: name: oci-native-ingress-controller-leader-election-role namespace: native-ingress-controller-system labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm rules: - apiGroups: ["coordination.k8s.io"] @@ -90,10 +90,10 @@ metadata: name: oci-native-ingress-controller-leader-election-rolebinding namespace: native-ingress-controller-system labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm roleRef: apiGroup: rbac.authorization.k8s.io diff --git a/deploy/manifests/oci-native-ingress-controller/templates/service.yaml b/deploy/manifests/oci-native-ingress-controller/templates/service.yaml index 5ef954e8..29b00f56 100644 --- a/deploy/manifests/oci-native-ingress-controller/templates/service.yaml +++ b/deploy/manifests/oci-native-ingress-controller/templates/service.yaml @@ -12,10 +12,10 @@ metadata: name: oci-native-ingress-controller namespace: native-ingress-controller-system labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm spec: type: ClusterIP diff --git a/deploy/manifests/oci-native-ingress-controller/templates/serviceaccount.yaml b/deploy/manifests/oci-native-ingress-controller/templates/serviceaccount.yaml index aeada9e1..b994a2f0 100644 --- a/deploy/manifests/oci-native-ingress-controller/templates/serviceaccount.yaml +++ b/deploy/manifests/oci-native-ingress-controller/templates/serviceaccount.yaml @@ -12,8 +12,8 @@ metadata: name: oci-native-ingress-controller namespace: native-ingress-controller-system labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm diff --git a/deploy/manifests/oci-native-ingress-controller/templates/webhook.yaml b/deploy/manifests/oci-native-ingress-controller/templates/webhook.yaml index 80220cda..68e9b617 100644 --- a/deploy/manifests/oci-native-ingress-controller/templates/webhook.yaml +++ b/deploy/manifests/oci-native-ingress-controller/templates/webhook.yaml @@ -36,10 +36,10 @@ kind: MutatingWebhookConfiguration metadata: name: oci-native-ingress-controller-webhook labels: - helm.sh/chart: oci-native-ingress-controller-1.0.0 + helm.sh/chart: oci-native-ingress-controller-1.2.0 app.kubernetes.io/name: oci-native-ingress-controller app.kubernetes.io/instance: oci-native-ingress-controller - app.kubernetes.io/version: "1.0.0" + app.kubernetes.io/version: "1.2.0" app.kubernetes.io/managed-by: Helm annotations: cert-manager.io/inject-ca-from: native-ingress-controller-system/oci-native-ingress-controller-webhook-serving-cert diff --git a/helm/oci-native-ingress-controller/Chart.yaml b/helm/oci-native-ingress-controller/Chart.yaml index 131a2b5f..51c428a6 100644 --- a/helm/oci-native-ingress-controller/Chart.yaml +++ b/helm/oci-native-ingress-controller/Chart.yaml @@ -8,8 +8,8 @@ apiVersion: v2 name: oci-native-ingress-controller description: OCI Native Ingress Controller type: application -version: 1.0.0 -appVersion: "1.0.0" +version: 1.2.0 +appVersion: "1.2.0" maintainers: - name: OKE Foundations team diff --git a/helm/oci-native-ingress-controller/values.yaml b/helm/oci-native-ingress-controller/values.yaml index edc1428f..88bbfba8 100644 --- a/helm/oci-native-ingress-controller/values.yaml +++ b/helm/oci-native-ingress-controller/values.yaml @@ -21,7 +21,7 @@ image: repository: ghcr.io/oracle/oci-native-ingress-controller pullPolicy: Always # Overrides the image tag whose default is the chart appVersion. - tag: "v1.0.0" + tag: "v1.2.0" imagePullSecrets: [] nameOverride: "" diff --git a/pkg/controllers/ingressclass/ingressclass.go b/pkg/controllers/ingressclass/ingressclass.go index ba70e0d0..464754e6 100644 --- a/pkg/controllers/ingressclass/ingressclass.go +++ b/pkg/controllers/ingressclass/ingressclass.go @@ -16,7 +16,6 @@ import ( "time" "github.com/oracle/oci-native-ingress-controller/pkg/client" - "github.com/oracle/oci-native-ingress-controller/pkg/exception" "k8s.io/klog/v2" ctrcache "sigs.k8s.io/controller-runtime/pkg/cache" @@ -196,22 +195,28 @@ func (c *Controller) sync(key string) error { func (c *Controller) getLoadBalancer(ic *networkingv1.IngressClass) (*ociloadbalancer.LoadBalancer, error) { lbID := util.GetIngressClassLoadBalancerId(ic) if lbID == "" { - return nil, &exception.NotFoundServiceError{} + klog.Errorf("LB id not set for ingressClass: %s", ic.Name) + return nil, nil // LoadBalancer ID not set, Trigger new LB creation } lb, _, err := c.client.GetLbClient().GetLoadBalancer(context.TODO(), lbID) if err != nil { + klog.Errorf("Error while fetching LB %s for ingressClass: %s, err: %s", lbID, ic.Name, err.Error()) + + // Check if Service error 404, then ignore it since LB is not found. + svcErr, ok := common.IsServiceError(err) + if ok && svcErr.GetHTTPStatusCode() == 404 { + return nil, nil // Redirect new LB creation + } return nil, err } - return lb, nil } func (c *Controller) ensureLoadBalancer(ic *networkingv1.IngressClass) error { lb, err := c.getLoadBalancer(ic) - svcErr, ok := common.IsServiceError(err) - if err != nil && (ok && svcErr.GetHTTPStatusCode() != 404) { + if err != nil { return err } diff --git a/pkg/controllers/ingressclass/ingressclass_test.go b/pkg/controllers/ingressclass/ingressclass_test.go index a4478539..55377858 100644 --- a/pkg/controllers/ingressclass/ingressclass_test.go +++ b/pkg/controllers/ingressclass/ingressclass_test.go @@ -9,6 +9,7 @@ import ( . "github.com/onsi/gomega" "github.com/oracle/oci-go-sdk/v65/common" ociloadbalancer "github.com/oracle/oci-go-sdk/v65/loadbalancer" + "github.com/oracle/oci-native-ingress-controller/pkg/exception" "github.com/oracle/oci-go-sdk/v65/waf" "github.com/oracle/oci-native-ingress-controller/pkg/client" @@ -38,6 +39,42 @@ func TestEnsureLoadBalancer(t *testing.T) { Expect(err).Should(BeNil()) } +func TestEnsureLoadBalancerWithLbIdSet(t *testing.T) { + RegisterTestingT(t) + ctx := context.TODO() + + ingressClassList := util.GetIngressClassListWithLBSet("id") + c := inits(ctx, ingressClassList) + + err := c.ensureLoadBalancer(&ingressClassList.Items[0]) + Expect(err).Should(BeNil()) +} + +func TestEnsureLoadBalancerWithNotFound(t *testing.T) { + RegisterTestingT(t) + ctx := context.TODO() + + ingressClassList := util.GetIngressClassListWithLBSet("notfound") + c := inits(ctx, ingressClassList) + + ic := &ingressClassList.Items[0] + err := c.ensureLoadBalancer(ic) + Expect(err).Should(BeNil()) + +} + +func TestEnsureLoadBalancerWithNetworkError(t *testing.T) { + RegisterTestingT(t) + ctx := context.TODO() + + ingressClassList := util.GetIngressClassListWithLBSet("networkerror") + c := inits(ctx, ingressClassList) + + err := c.ensureLoadBalancer(&ingressClassList.Items[0]) + Expect(err).Should(Not(BeNil())) + Expect(err.Error()).Should(Equal("Failure due to network error")) +} + func TestIngressClassAdd(t *testing.T) { RegisterTestingT(t) ctx, cancel := context.WithCancel(context.Background()) @@ -245,10 +282,24 @@ type MockLoadBalancerClient struct { } func (m MockLoadBalancerClient) GetLoadBalancer(ctx context.Context, request ociloadbalancer.GetLoadBalancerRequest) (ociloadbalancer.GetLoadBalancerResponse, error) { + if *request.LoadBalancerId == "networkerror" { + return ociloadbalancer.GetLoadBalancerResponse{}, NetworkError{} + } + if *request.LoadBalancerId == "notfound" { + return ociloadbalancer.GetLoadBalancerResponse{}, &exception.NotFoundServiceError{} + } + res := util.SampleLoadBalancerResponse() return res, nil } +type NetworkError struct { +} + +func (n NetworkError) Error() string { + return "Failure due to network error" +} + func (m MockLoadBalancerClient) UpdateLoadBalancer(ctx context.Context, request ociloadbalancer.UpdateLoadBalancerRequest) (response ociloadbalancer.UpdateLoadBalancerResponse, err error) { return ociloadbalancer.UpdateLoadBalancerResponse{ RawResponse: nil,