From a06199ae3683fc448271ab6a9703eb0c609f99cd Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 29 May 2024 18:42:37 +0300 Subject: [PATCH 1/2] handle default ingress class in flags --- internal/flags/flags.go | 6 +++++- internal/ingress/utils.go | 8 ++------ internal/ingress/utils_test.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 22ad19c..fc425e6 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -13,6 +13,10 @@ import ( k8sopts "k8s.io/component-base/config/options" ) +const ( + DefaultScIngressClass = "serverscom" +) + // ParseFlags parses os args and map them to controller configuration func ParseFlags() (*controller.Configuration, error) { var ( @@ -24,7 +28,7 @@ func ParseFlags() (*controller.Configuration, error) { watchNamespace = flags.String("watch-namespace", v1.NamespaceAll, `Namespace to watch for Ingress/Services/Endpoints.`) - ingressClass = flags.String("ingress-class", "", + ingressClass = flags.String("ingress-class", DefaultScIngressClass, `If set, overrides what ingress classes are managed by the controller.`) resyncPeriod = flags.Duration("sync-period", 0, diff --git a/internal/ingress/utils.go b/internal/ingress/utils.go index 6316496..b51e209 100644 --- a/internal/ingress/utils.go +++ b/internal/ingress/utils.go @@ -3,15 +3,11 @@ package ingress import v1 "k8s.io/api/networking/v1" const ( - IngressClassKey = "kubernetes.io/ingress.class" - DefaultScIngressClass = "serverscom" + IngressClassKey = "kubernetes.io/ingress.class" ) -// IsScIngress checks if Ingress belongs to a specified class or DefaultScIngressClass if class empty +// IsScIngress checks if Ingress belongs to a specified class func IsScIngress(i *v1.Ingress, class string) bool { - if class == "" { - class = DefaultScIngressClass - } if i.Spec.IngressClassName != nil { return *i.Spec.IngressClassName == class } diff --git a/internal/ingress/utils_test.go b/internal/ingress/utils_test.go index 4f9b21b..36f4b9a 100644 --- a/internal/ingress/utils_test.go +++ b/internal/ingress/utils_test.go @@ -11,7 +11,7 @@ import ( func TestIsScIngress(t *testing.T) { g := NewWithT(t) - defaultClass := DefaultScIngressClass + defaultClass := "serverscom" otherClass := "other-class" g.Expect(IsScIngress(&v1.Ingress{}, "")).To(BeFalse()) From 34c5d88c1e9cc1fe8ad3938a22a654583c6cc4db Mon Sep 17 00:00:00 2001 From: Oleg Isakov Date: Wed, 29 May 2024 19:11:01 +0300 Subject: [PATCH 2/2] improve logging; update tests --- env.example | 2 +- internal/ingress/controller/store/store.go | 2 +- internal/ingress/controller/store/store_test.go | 11 ++++++++++- internal/ingress/controller/store/utils.go | 2 ++ internal/service/loadbalancer/manager.go | 5 +++++ internal/service/loadbalancer/manager_test.go | 7 +++---- internal/service/service.go | 4 +++- internal/service/service_test.go | 4 ++-- 8 files changed, 27 insertions(+), 10 deletions(-) diff --git a/env.example b/env.example index c4c7875..8908b65 100644 --- a/env.example +++ b/env.example @@ -8,4 +8,4 @@ SC_ACCESS_TOKEN=jwt_token SC_LOCATION_ID=1 # if missing or empty prod api url will be used -SC_API_URL=https://api.dev1.privateservergrid.com/v1 +SC_API_URL= diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index ea7034f..81ea933 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -74,7 +74,7 @@ func (s *Store) GetNodesIpList() []string { return s.listers.Node.NodesIpList() } -// GetSecret returns the Secret matching key. +// GetIngressServiceInfo returns ingress services info. func (s *Store) GetIngressServiceInfo(ingress *networkv1.Ingress) (map[string]ServiceInfo, error) { return getIngressServiceInfo(ingress, s) } diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index e887522..5e97221 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -1,6 +1,7 @@ package store import ( + "errors" "testing" "time" @@ -11,7 +12,7 @@ import ( ) var ( - scIngressClassName = "sc-ingress" + scIngressClassName = "serverscom" nonScIngressClassName = "not-sc-ingress" scIngress = &networkv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -200,4 +201,12 @@ func TestGetIngressServiceInfo(t *testing.T) { g.Expect(serviceInfo[serviceName].NodePort).To(Equal(30000)) g.Expect(serviceInfo[serviceName].NodeIps).To(ConsistOf("192.168.1.1")) g.Expect(serviceInfo[serviceName].Annotations).To(HaveKeyWithValue("key", "value")) + + // check if service doens't have NodePort + service.Spec.Ports[0].NodePort = 0 + s.listers.Service.Update(service) + + serviceInfo, err = s.GetIngressServiceInfo(ingress) + expectedErr := errors.New("service doesn't have NodePort, only services with type 'NodePort' or 'LoadBalancer' supported") + g.Expect(err).To(Equal(expectedErr)) } diff --git a/internal/ingress/controller/store/utils.go b/internal/ingress/controller/store/utils.go index 27a9aea..cf93e9b 100644 --- a/internal/ingress/controller/store/utils.go +++ b/internal/ingress/controller/store/utils.go @@ -46,6 +46,8 @@ func getIngressServiceInfo(ingress *networkv1.Ingress, store Storer) (map[string sTmp.Hosts = append(sTmp.Hosts, rule.Host) servicesInfo[serviceName] = sTmp } + } else { + return nil, fmt.Errorf("service doesn't have NodePort, only services with type 'NodePort' or 'LoadBalancer' supported") } break } diff --git a/internal/service/loadbalancer/manager.go b/internal/service/loadbalancer/manager.go index 73554ba..8eff12c 100644 --- a/internal/service/loadbalancer/manager.go +++ b/internal/service/loadbalancer/manager.go @@ -1,6 +1,7 @@ package loadbalancer import ( + "errors" "fmt" "strconv" "sync" @@ -197,6 +198,10 @@ func (m *Manager) TranslateIngressToLB(ingress *networkv1.Ingress, sslCerts map[ uZInput = *annotations.FillLBUpstreamZoneWithServiceAnnotations(&uZInput, service.Annotations) upstreamZones = append(upstreamZones, uZInput) } + if len(vhostZones) == 0 || len(upstreamZones) == 0 { + return nil, errors.New("vhost or upstream can't be empty, can't continue") + } + locIdStr := config.FetchEnv("SC_LOCATION_ID", "1") locId, err := strconv.Atoi(locIdStr) if err != nil { diff --git a/internal/service/loadbalancer/manager_test.go b/internal/service/loadbalancer/manager_test.go index ee15845..871e6dd 100644 --- a/internal/service/loadbalancer/manager_test.go +++ b/internal/service/loadbalancer/manager_test.go @@ -296,10 +296,9 @@ func TestTranslateIngressToLB(t *testing.T) { g := NewWithT(t) storeHandler.EXPECT().GetIngressServiceInfo(ingress).Return(make(map[string]store.ServiceInfo), nil) lbInput, err := manager.TranslateIngressToLB(ingress, sslCerts) - g.Expect(err).To(BeNil()) - g.Expect(lbInput).NotTo(BeNil()) - g.Expect(lbInput.UpstreamZones).To(BeEmpty()) - g.Expect(lbInput.VHostZones).To(BeEmpty()) + expectedErr := errors.New("vhost or upstream can't be empty, can't continue") + g.Expect(err).To(Equal(expectedErr)) + g.Expect(lbInput).To(BeNil()) }) } diff --git a/internal/service/service.go b/internal/service/service.go index 10972a2..c9e5687 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -138,9 +138,11 @@ func (s *Service) SyncToPortal(key string) error { if err != nil { s.recorder.Eventf(ing, v1.EventTypeWarning, "UpdateStatus", err.Error()) } + + s.recorder.Eventf(ing, v1.EventTypeNormal, "Synced", "Successfully synced") }() - s.recorder.Eventf(ing, v1.EventTypeNormal, "Synced", "Successfully synced to portal") + s.recorder.Eventf(ing, v1.EventTypeNormal, "Created", "Successfully created") return nil } diff --git a/internal/service/service_test.go b/internal/service/service_test.go index f2684f3..05eb141 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -179,7 +179,7 @@ func TestSyncToPortal(t *testing.T) { } } g.Expect(events).To(ContainElements( - "Normal Synced Successfully synced to portal", + "Normal Synced Successfully synced", `Warning UpdateStatus ingresses.networking.k8s.io "test-ingress" not found`, )) }) @@ -209,7 +209,7 @@ func TestSyncToPortal(t *testing.T) { select { case e := <-recorder.Events: - expectedEvent := "Normal Synced Successfully synced to portal" + expectedEvent := "Normal Created Successfully created" g.Expect(e).To(BeEquivalentTo(expectedEvent)) case <-time.After(time.Second * 1): t.Fatal("Timeout waiting for event")