diff --git a/cloudstack_loadbalancer.go b/cloudstack_loadbalancer.go index ffbdd7cd..48e8ce58 100644 --- a/cloudstack_loadbalancer.go +++ b/cloudstack_loadbalancer.go @@ -56,6 +56,9 @@ const ( // associated the IP address. This annotation is set by the controller when it associates // an unallocated IP, and is used to determine if the IP should be disassociated on deletion. ServiceAnnotationLoadBalancerIPAssociatedByController = "service.beta.kubernetes.io/cloudstack-load-balancer-ip-associated-by-controller" //nolint:gosec + + ServiceAnnotationLoadBalancerStickynessMethodName = "service.beta.kubernetes.io/cloudstack-load-balancer-stickyness-method-name" + ServiceAnnotationLoadBalancerStickynessParam = "service.beta.kubernetes.io/cloudstack-load-balancer-stickyness-method-param" ) type loadBalancer struct { @@ -69,6 +72,7 @@ type loadBalancer struct { networkID string projectID string rules map[string]*cloudstack.LoadBalancerRule + stickynessPolicies map[string]*cloudstack.LBStickinessPolicyStickinesspolicy ipAssociatedByController bool } @@ -181,12 +185,36 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s // Delete the rule from the map, to prevent it being deleted. delete(lb.rules, lbRuleName) } + + stickynessPolicy, stickynessPolicyNeedsUpdate, err := lb.checkStickynessPolicy(lbRule, service) + if err != nil { + return nil, err + } + if stickynessPolicyNeedsUpdate { + if stickynessPolicy != nil { + klog.V(4).Infof("Recreate stickyness policy: %v", lbRuleName) + if err := lb.deleteStickynessPolicy(stickynessPolicy.Id); err != nil { + return nil, err + } + delete(lb.stickynessPolicies, lbRule.Id) + } else { + klog.V(4).Infof("Creating stickyness policy: %v", lbRuleName) + } + if _, err := lb.createStickynessPolicy(lbRuleName, lbRule.Id, service); err != nil { + return nil, err + } + // Remove from map to mark as handled (map tracks initial state for comparison) + delete(lb.stickynessPolicies, lbRule.Id) + } } else { klog.V(4).Infof("Creating load balancer rule: %v", lbRuleName) lbRule, err = lb.createLoadBalancerRule(lbRuleName, port, protocol, service) if err != nil { return nil, err } + if _, err := lb.createStickynessPolicy(lbRuleName, lbRule.Id, service); err != nil { + return nil, err + } klog.V(4).Infof("Assigning hosts (%v) to load balancer rule: %v", lb.hostIDs, lbRuleName) if err = lb.assignHostsToRule(lbRule, lb.hostIDs); err != nil { @@ -434,10 +462,11 @@ func (cs *CSCloud) GetLoadBalancerName(ctx context.Context, clusterName string, // getLoadBalancer retrieves the IP address and ID and all the existing rules it can find. func (cs *CSCloud) getLoadBalancer(service *corev1.Service) (*loadBalancer, error) { lb := &loadBalancer{ - CloudStackClient: cs.client, - name: cs.GetLoadBalancerName(context.TODO(), "", service), - projectID: cs.projectID, - rules: make(map[string]*cloudstack.LoadBalancerRule), + CloudStackClient: cs.client, + name: cs.GetLoadBalancerName(context.TODO(), "", service), + projectID: cs.projectID, + rules: make(map[string]*cloudstack.LoadBalancerRule), + stickynessPolicies: make(map[string]*cloudstack.LBStickinessPolicyStickinesspolicy), } p := cs.client.LoadBalancer.NewListLoadBalancerRulesParams() @@ -462,6 +491,19 @@ func (cs *CSCloud) getLoadBalancer(service *corev1.Service) (*loadBalancer, erro lb.ipAddr = lbRule.Publicip lb.ipAddrID = lbRule.Publicipid + + lbStickinessPoliciesParams := cs.client.LoadBalancer.NewListLBStickinessPoliciesParams() + lbStickinessPoliciesParams.SetLbruleid(lbRule.Id) + lbStickinessPolicies, err := cs.client.LoadBalancer.ListLBStickinessPolicies(lbStickinessPoliciesParams) + if err != nil { + return nil, fmt.Errorf("error retrieving stickyness policies: %v", err) + } + // CloudStack returns a policy wrapper per rule even when the rule has no + // stickyness policy, with an empty Stickinesspolicy list inside it. + if len(lbStickinessPolicies.LBStickinessPolicies) > 0 && + len(lbStickinessPolicies.LBStickinessPolicies[0].Stickinesspolicy) > 0 { + lb.stickynessPolicies[lbRule.Id] = &lbStickinessPolicies.LBStickinessPolicies[0].Stickinesspolicy[0] + } } klog.V(4).Infof("Load balancer %v contains %d rule(s)", lb.name, len(lb.rules)) @@ -649,6 +691,60 @@ func (lb *loadBalancer) getCIDRList(service *corev1.Service) ([]string, error) { return cidrList, nil } +func (lb *loadBalancer) checkStickynessPolicy(lbRule *cloudstack.LoadBalancerRule, service *corev1.Service) (*cloudstack.LBStickinessPolicyStickinesspolicy, bool, error) { + stickynessPolicy := lb.stickynessPolicies[lbRule.Id] + stickynessMethodName := getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerStickynessMethodName, "") + stickynessMethodParam := getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerStickynessParam, "") + stickynessMethodParams := parseStickynessParams(stickynessMethodParam) + + // If no policy exists and no method name is specified, no action needed + if stickynessPolicy == nil { + if stickynessMethodName == "" { + return nil, false, nil + } + klog.V(4).Infof("sticky policy not found for rule: %v", lbRule.Name) + return nil, true, nil + } + + // If policy exists but method name is not specified, policy should be deleted + if stickynessMethodName == "" { + klog.V(4).Infof("sticky policy exists but annotation removed for rule: %v", lbRule.Name) + return stickynessPolicy, true, nil + } + + // Policy exists and method name is specified - check if it matches + klog.V(4).Infof("sticky policy found for rule: %v", lbRule.Name) + if stickynessPolicy.Methodname != stickynessMethodName { + klog.V(4).Infof("sticky policy method name does not match: %v", lbRule.Name) + return stickynessPolicy, true, nil + } + + // Check if params match + if len(stickynessPolicy.Params) != len(stickynessMethodParams) { + klog.V(4).Infof("sticky policy params length does not match: %v", lbRule.Name) + return stickynessPolicy, true, nil + } + + // Check if all keys in stickynessPolicy.Params match stickynessMethodParams + for key, value := range stickynessPolicy.Params { + if stickynessMethodParams[key] != value { + klog.V(4).Infof("sticky policy param %v does not match: %v", key, value) + return stickynessPolicy, true, nil + } + } + + // Check if all keys in stickynessMethodParams exist in stickynessPolicy.Params + for key := range stickynessMethodParams { + if _, exists := stickynessPolicy.Params[key]; !exists { + klog.V(4).Infof("sticky policy missing param: %v", key) + return stickynessPolicy, true, nil + } + } + + // Policy matches desired state + return stickynessPolicy, false, nil +} + // checkLoadBalancerRule checks if the rule already exists and if it does, if it can be updated. If // it does exist but cannot be updated, it will delete the existing rule so it can be created again. func (lb *loadBalancer) checkLoadBalancerRule(lbRuleName string, port corev1.ServicePort, protocol LoadBalancerProtocol, service *corev1.Service, version semver.Version) (*cloudstack.LoadBalancerRule, bool, error) { @@ -715,6 +811,45 @@ func (lb *loadBalancer) updateLoadBalancerRule(lbRuleName string, protocol LoadB return err } +// createStickynessPolicy creates a new stickyness policy and returns it. +func (lb *loadBalancer) createStickynessPolicy(lbRuleName string, lbRuleId string, service *corev1.Service) (*cloudstack.LBStickinessPolicyStickinesspolicy, error) { + stickynessMethodName := getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerStickynessMethodName, "") + stickynessMethodParam := getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerStickynessParam, "") + // If the stickyness method name is not set, we don't need to create a stickyness policy. + if stickynessMethodName == "" { + return nil, nil + } + p := lb.LoadBalancer.NewCreateLBStickinessPolicyParams(lbRuleId, stickynessMethodName, lbRuleName) + + params := parseStickynessParams(stickynessMethodParam) + p.SetParam(params) + + stickynessPolicy, err := lb.LoadBalancer.CreateLBStickinessPolicy(p) + if err != nil { + return nil, fmt.Errorf("error creating stickyness policy: %v", err) + } + if len(stickynessPolicy.Stickinesspolicy) == 0 { + return nil, fmt.Errorf("error creating stickyness policy: no policy returned for load balancer rule %v", lbRuleName) + } + return &cloudstack.LBStickinessPolicyStickinesspolicy{ + Methodname: stickynessPolicy.Stickinesspolicy[0].Methodname, + Params: stickynessPolicy.Stickinesspolicy[0].Params, + Id: stickynessPolicy.Stickinesspolicy[0].Id, + Name: stickynessPolicy.Stickinesspolicy[0].Name, + State: stickynessPolicy.Stickinesspolicy[0].State, + }, nil +} + +// deleteStickynessPolicy deletes a stickyness policy. +func (lb *loadBalancer) deleteStickynessPolicy(stickynessPolicyId string) error { + p := lb.LoadBalancer.NewDeleteLBStickinessPolicyParams(stickynessPolicyId) + + if _, err := lb.LoadBalancer.DeleteLBStickinessPolicy(p); err != nil { + return fmt.Errorf("error deleting stickyness policy %v: %v", stickynessPolicyId, err) + } + return nil +} + // createLoadBalancerRule creates a new load balancer rule and returns it's ID. func (lb *loadBalancer) createLoadBalancerRule(lbRuleName string, port corev1.ServicePort, protocol LoadBalancerProtocol, service *corev1.Service) (*cloudstack.LoadBalancerRule, error) { p := lb.LoadBalancer.NewCreateLoadBalancerRuleParams( @@ -772,6 +907,7 @@ func (lb *loadBalancer) deleteLoadBalancerRule(lbRule *cloudstack.LoadBalancerRu // Delete the rule from the map as it no longer exists delete(lb.rules, lbRule.Name) + delete(lb.stickynessPolicies, lbRule.Id) return nil } @@ -1136,6 +1272,23 @@ func getStringFromServiceAnnotation(service *corev1.Service, annotationKey strin return defaultSetting } +// parseStickynessParams parses a comma-separated string of key=value pairs into a map. +// Empty values and malformed entries are ignored. +func parseStickynessParams(paramString string) map[string]string { + params := make(map[string]string) + for _, param := range strings.Split(paramString, ",") { + param = strings.TrimSpace(param) + if param == "" { + continue + } + parts := strings.SplitN(param, "=", 2) + if len(parts) == 2 { + params[parts[0]] = parts[1] + } + } + return params +} + // getBoolFromServiceAnnotation searches a given v1.Service for a specific annotationKey and either returns the annotation's boolean value or a specified defaultSetting func getBoolFromServiceAnnotation(service *corev1.Service, annotationKey string, defaultSetting bool) bool { klog.V(4).Infof("getBoolFromServiceAnnotation(%s/%s, %v, %v)", service.Namespace, service.Name, annotationKey, defaultSetting) diff --git a/cloudstack_loadbalancer_test.go b/cloudstack_loadbalancer_test.go index 4bbf38e7..f8fc5ba8 100644 --- a/cloudstack_loadbalancer_test.go +++ b/cloudstack_loadbalancer_test.go @@ -3277,9 +3277,35 @@ func TestGetLoadBalancer(t *testing.T) { }, } + // rule-1 has a stickyness policy configured, rule-2 has none. + stickyResp := &cloudstack.ListLBStickinessPoliciesResponse{ + Count: 1, + LBStickinessPolicies: []*cloudstack.LBStickinessPolicy{ + { + Lbruleid: "rule-1", + Stickinesspolicy: []cloudstack.LBStickinessPolicyStickinesspolicy{ + { + Id: "policy-1", + Name: "test-service-tcp-80", + Methodname: "LbCookie", + Params: map[string]string{"cookie-name": "SERVERID"}, + }, + }, + }, + }, + } + emptyStickyResp := &cloudstack.ListLBStickinessPoliciesResponse{ + Count: 0, + LBStickinessPolicies: []*cloudstack.LBStickinessPolicy{}, + } + gomock.InOrder( mockLB.EXPECT().NewListLoadBalancerRulesParams().Return(listParams), mockLB.EXPECT().ListLoadBalancerRules(gomock.Any()).Return(listResp, nil), + mockLB.EXPECT().NewListLBStickinessPoliciesParams().Return(&cloudstack.ListLBStickinessPoliciesParams{}), + mockLB.EXPECT().ListLBStickinessPolicies(gomock.Any()).Return(stickyResp, nil), + mockLB.EXPECT().NewListLBStickinessPoliciesParams().Return(&cloudstack.ListLBStickinessPoliciesParams{}), + mockLB.EXPECT().ListLBStickinessPolicies(gomock.Any()).Return(emptyStickyResp, nil), ) cs := &CSCloud{ @@ -3299,6 +3325,14 @@ func TestGetLoadBalancer(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + if len(lb.stickynessPolicies) != 1 { + t.Errorf("stickynessPolicies count = %d, want %d", len(lb.stickynessPolicies), 1) + } + if policy, ok := lb.stickynessPolicies["rule-1"]; !ok { + t.Errorf("stickynessPolicies missing entry for %q", "rule-1") + } else if policy.Id != "policy-1" { + t.Errorf("stickyness policy ID = %q, want %q", policy.Id, "policy-1") + } if lb.ipAddr != "203.0.113.1" { t.Errorf("ipAddr = %q, want %q", lb.ipAddr, "203.0.113.1") } @@ -3685,3 +3719,488 @@ func TestVerifyHosts(t *testing.T) { } }) } + +func TestParseStickynessParams(t *testing.T) { + tests := []struct { + name string + input string + want map[string]string + }{ + { + name: "empty string returns empty map", + input: "", + want: map[string]string{}, + }, + { + name: "single pair", + input: "cookie-name=SERVERID", + want: map[string]string{"cookie-name": "SERVERID"}, + }, + { + name: "multiple pairs", + input: "cookie-name=SERVERID,mode=insert", + want: map[string]string{"cookie-name": "SERVERID", "mode": "insert"}, + }, + { + name: "whitespace around entries is trimmed", + input: " cookie-name=SERVERID , mode=insert ", + want: map[string]string{"cookie-name": "SERVERID", "mode": "insert"}, + }, + { + name: "entry without separator is ignored", + input: "cookie-name=SERVERID,bogus", + want: map[string]string{"cookie-name": "SERVERID"}, + }, + { + name: "trailing comma is ignored", + input: "mode=insert,", + want: map[string]string{"mode": "insert"}, + }, + { + name: "only separators returns empty map", + input: ",,,", + want: map[string]string{}, + }, + { + name: "empty value is preserved", + input: "cookie-name=", + want: map[string]string{"cookie-name": ""}, + }, + { + name: "value containing separator is kept intact", + input: "expr=a=b", + want: map[string]string{"expr": "a=b"}, + }, + { + name: "duplicate key keeps last value", + input: "mode=insert,mode=rewrite", + want: map[string]string{"mode": "rewrite"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseStickynessParams(tt.input) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("parseStickynessParams(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +// stickynessTestService builds a service carrying the stickyness annotations. An +// empty method or param string omits that annotation entirely. +func stickynessTestService(methodName, params string) *corev1.Service { + annotations := map[string]string{} + if methodName != "" { + annotations[ServiceAnnotationLoadBalancerStickynessMethodName] = methodName + } + if params != "" { + annotations[ServiceAnnotationLoadBalancerStickynessParam] = params + } + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Namespace: "default", + Annotations: annotations, + }, + } +} + +func TestCheckStickynessPolicy(t *testing.T) { + lbRule := &cloudstack.LoadBalancerRule{Id: "rule-id", Name: "test-service-tcp-80"} + + tests := []struct { + name string + existingPolicy *cloudstack.LBStickinessPolicyStickinesspolicy + methodName string + params string + wantPolicy bool // true when the existing policy is expected back + wantNeedsUpdate bool + }{ + { + name: "no policy and no annotation is a no-op", + existingPolicy: nil, + methodName: "", + wantPolicy: false, + wantNeedsUpdate: false, + }, + { + name: "no policy with annotation needs creation", + existingPolicy: nil, + methodName: "LbCookie", + wantPolicy: false, + wantNeedsUpdate: true, + }, + { + name: "policy with annotation removed needs deletion", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "LbCookie", + }, + methodName: "", + wantPolicy: true, + wantNeedsUpdate: true, + }, + { + name: "matching method with no params is up-to-date", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "LbCookie", + Params: map[string]string{}, + }, + methodName: "LbCookie", + params: "", + wantPolicy: true, + wantNeedsUpdate: false, + }, + { + name: "matching method and params is up-to-date", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "AppCookie", + Params: map[string]string{"cookie-name": "SERVERID", "mode": "insert"}, + }, + methodName: "AppCookie", + params: "cookie-name=SERVERID,mode=insert", + wantPolicy: true, + wantNeedsUpdate: false, + }, + { + name: "method name mismatch needs recreation", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "LbCookie", + }, + methodName: "AppCookie", + wantPolicy: true, + wantNeedsUpdate: true, + }, + { + name: "extra desired param needs recreation", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "AppCookie", + Params: map[string]string{"cookie-name": "SERVERID"}, + }, + methodName: "AppCookie", + params: "cookie-name=SERVERID,mode=insert", + wantPolicy: true, + wantNeedsUpdate: true, + }, + { + name: "removed desired param needs recreation", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "AppCookie", + Params: map[string]string{"cookie-name": "SERVERID", "mode": "insert"}, + }, + methodName: "AppCookie", + params: "cookie-name=SERVERID", + wantPolicy: true, + wantNeedsUpdate: true, + }, + { + name: "param value mismatch needs recreation", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "AppCookie", + Params: map[string]string{"cookie-name": "SERVERID"}, + }, + methodName: "AppCookie", + params: "cookie-name=JSESSIONID", + wantPolicy: true, + wantNeedsUpdate: true, + }, + { + name: "renamed param key of equal count needs recreation", + existingPolicy: &cloudstack.LBStickinessPolicyStickinesspolicy{ + Id: "policy-id", + Methodname: "AppCookie", + Params: map[string]string{"cookie-name": ""}, + }, + methodName: "AppCookie", + params: "mode=", + wantPolicy: true, + wantNeedsUpdate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lb := &loadBalancer{ + stickynessPolicies: map[string]*cloudstack.LBStickinessPolicyStickinesspolicy{}, + } + if tt.existingPolicy != nil { + lb.stickynessPolicies[lbRule.Id] = tt.existingPolicy + } + + policy, needsUpdate, err := lb.checkStickynessPolicy(lbRule, stickynessTestService(tt.methodName, tt.params)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.wantPolicy && policy != tt.existingPolicy { + t.Errorf("policy = %v, want the existing policy %v", policy, tt.existingPolicy) + } + if !tt.wantPolicy && policy != nil { + t.Errorf("policy = %v, want nil", policy) + } + if needsUpdate != tt.wantNeedsUpdate { + t.Errorf("needsUpdate = %v, want %v", needsUpdate, tt.wantNeedsUpdate) + } + }) + } +} + +func TestCreateStickynessPolicy(t *testing.T) { + t.Run("no method annotation is a no-op", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + // No expectations on the mock; any API call would fail the test. + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, + }, + } + + policy, err := lb.createStickynessPolicy("test-service-tcp-80", "rule-id", stickynessTestService("", "")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if policy != nil { + t.Errorf("policy = %v, want nil", policy) + } + }) + + t.Run("creates policy from annotations", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + createParams := &cloudstack.CreateLBStickinessPolicyParams{} + createResp := &cloudstack.CreateLBStickinessPolicyResponse{ + Lbruleid: "rule-id", + Stickinesspolicy: []cloudstack.CreateLBStickinessPolicyResponseStickinesspolicy{ + { + Id: "policy-id", + Name: "test-service-tcp-80", + Methodname: "AppCookie", + Params: map[string]string{"cookie-name": "SERVERID"}, + State: "Active", + }, + }, + } + + gomock.InOrder( + mockLB.EXPECT().NewCreateLBStickinessPolicyParams("rule-id", "AppCookie", "test-service-tcp-80").Return(createParams), + mockLB.EXPECT().CreateLBStickinessPolicy(createParams).Return(createResp, nil), + ) + + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, + }, + } + + policy, err := lb.createStickynessPolicy("test-service-tcp-80", "rule-id", stickynessTestService("AppCookie", "cookie-name=SERVERID")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if policy == nil { + t.Fatal("expected a policy, got nil") + } + if policy.Id != "policy-id" { + t.Errorf("policy ID = %q, want %q", policy.Id, "policy-id") + } + if policy.Methodname != "AppCookie" { + t.Errorf("policy method = %q, want %q", policy.Methodname, "AppCookie") + } + if policy.Name != "test-service-tcp-80" { + t.Errorf("policy name = %q, want %q", policy.Name, "test-service-tcp-80") + } + if policy.State != "Active" { + t.Errorf("policy state = %q, want %q", policy.State, "Active") + } + if !reflect.DeepEqual(policy.Params, map[string]string{"cookie-name": "SERVERID"}) { + t.Errorf("policy params = %v, want %v", policy.Params, map[string]string{"cookie-name": "SERVERID"}) + } + }) + + t.Run("API error is returned", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + createParams := &cloudstack.CreateLBStickinessPolicyParams{} + apiErr := fmt.Errorf("create policy API error") + + gomock.InOrder( + mockLB.EXPECT().NewCreateLBStickinessPolicyParams("rule-id", "LbCookie", "test-service-tcp-80").Return(createParams), + mockLB.EXPECT().CreateLBStickinessPolicy(createParams).Return(nil, apiErr), + ) + + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, + }, + } + + policy, err := lb.createStickynessPolicy("test-service-tcp-80", "rule-id", stickynessTestService("LbCookie", "")) + if err == nil { + t.Fatal("expected an error, got nil") + } + if policy != nil { + t.Errorf("policy = %v, want nil", policy) + } + if !strings.Contains(err.Error(), "error creating stickyness policy") { + t.Errorf("error = %q, want it to mention creating the stickyness policy", err.Error()) + } + }) +} + +func TestDeleteStickynessPolicy(t *testing.T) { + t.Run("deletes policy by ID", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + deleteParams := &cloudstack.DeleteLBStickinessPolicyParams{} + + gomock.InOrder( + mockLB.EXPECT().NewDeleteLBStickinessPolicyParams("policy-id").Return(deleteParams), + mockLB.EXPECT().DeleteLBStickinessPolicy(deleteParams).Return(&cloudstack.DeleteLBStickinessPolicyResponse{Success: true}, nil), + ) + + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, + }, + } + + if err := lb.deleteStickynessPolicy("policy-id"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("API error is returned", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + deleteParams := &cloudstack.DeleteLBStickinessPolicyParams{} + apiErr := fmt.Errorf("delete policy API error") + + gomock.InOrder( + mockLB.EXPECT().NewDeleteLBStickinessPolicyParams("policy-id").Return(deleteParams), + mockLB.EXPECT().DeleteLBStickinessPolicy(deleteParams).Return(nil, apiErr), + ) + + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, + }, + } + + err := lb.deleteStickynessPolicy("policy-id") + if err == nil { + t.Fatal("expected an error, got nil") + } + if !strings.Contains(err.Error(), "error deleting stickyness policy policy-id") { + t.Errorf("error = %q, want it to mention deleting the stickyness policy", err.Error()) + } + }) +} + +// TestGetLoadBalancerEmptyStickynessPolicyList covers the shape CloudStack +// actually returns for a rule with no stickyness policy: a policy wrapper with +// an empty inner Stickinesspolicy list. +func TestGetLoadBalancerEmptyStickynessPolicyList(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + listResp := &cloudstack.ListLoadBalancerRulesResponse{ + Count: 1, + LoadBalancerRules: []*cloudstack.LoadBalancerRule{ + {Id: "rule-1", Name: "test-service-tcp-80", Publicip: "203.0.113.1", Publicipid: "ip-123"}, + }, + } + stickyResp := &cloudstack.ListLBStickinessPoliciesResponse{ + Count: 1, + LBStickinessPolicies: []*cloudstack.LBStickinessPolicy{ + { + Lbruleid: "rule-1", + Stickinesspolicy: []cloudstack.LBStickinessPolicyStickinesspolicy{}, + }, + }, + } + + gomock.InOrder( + mockLB.EXPECT().NewListLoadBalancerRulesParams().Return(&cloudstack.ListLoadBalancerRulesParams{}), + mockLB.EXPECT().ListLoadBalancerRules(gomock.Any()).Return(listResp, nil), + mockLB.EXPECT().NewListLBStickinessPoliciesParams().Return(&cloudstack.ListLBStickinessPoliciesParams{}), + mockLB.EXPECT().ListLBStickinessPolicies(gomock.Any()).Return(stickyResp, nil), + ) + + cs := &CSCloud{ + client: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, + }, + } + service := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Namespace: "default", + }, + } + + lb, err := cs.getLoadBalancer(service) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(lb.rules) != 1 { + t.Errorf("rules count = %d, want %d", len(lb.rules), 1) + } + if len(lb.stickynessPolicies) != 0 { + t.Errorf("stickynessPolicies count = %d, want %d", len(lb.stickynessPolicies), 0) + } +} + +// TestCreateStickynessPolicyEmptyResponse ensures an empty create response is +// reported as an error rather than dereferenced. +func TestCreateStickynessPolicyEmptyResponse(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + createParams := &cloudstack.CreateLBStickinessPolicyParams{} + createResp := &cloudstack.CreateLBStickinessPolicyResponse{ + Lbruleid: "rule-id", + Stickinesspolicy: []cloudstack.CreateLBStickinessPolicyResponseStickinesspolicy{}, + } + + gomock.InOrder( + mockLB.EXPECT().NewCreateLBStickinessPolicyParams("rule-id", "LbCookie", "test-service-tcp-80").Return(createParams), + mockLB.EXPECT().CreateLBStickinessPolicy(createParams).Return(createResp, nil), + ) + + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: mockLB, + }, + } + + policy, err := lb.createStickynessPolicy("test-service-tcp-80", "rule-id", stickynessTestService("LbCookie", "")) + if err == nil { + t.Fatal("expected an error, got nil") + } + if policy != nil { + t.Errorf("policy = %v, want nil", policy) + } + if !strings.Contains(err.Error(), "no policy returned") { + t.Errorf("error = %q, want it to mention that no policy was returned", err.Error()) + } +} diff --git a/go.mod b/go.mod index 24e177ad..be0b8af9 100644 --- a/go.mod +++ b/go.mod @@ -3,13 +3,14 @@ module github.com/apache/cloudstack-kubernetes-provider go 1.23.0 require ( - github.com/apache/cloudstack-go/v2 v2.19.0 + github.com/apache/cloudstack-go/v2 v2.19.1 github.com/blang/semver/v4 v4.0.0 github.com/spf13/pflag v1.0.5 go.uber.org/mock v0.5.0 gopkg.in/gcfg.v1 v1.2.3 k8s.io/api v0.24.17 k8s.io/apimachinery v0.24.17 + k8s.io/client-go v0.24.17 k8s.io/cloud-provider v0.24.17 k8s.io/component-base v0.24.17 k8s.io/klog/v2 v2.80.1 @@ -95,7 +96,6 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiserver v0.24.17 // indirect - k8s.io/client-go v0.24.17 // indirect k8s.io/component-helpers v0.24.17 // indirect k8s.io/controller-manager v0.24.17 // indirect k8s.io/kube-openapi v0.0.0-20220328201542-3ee0da9b0b42 // indirect diff --git a/go.sum b/go.sum index 85fd3e0f..423e8ea9 100644 --- a/go.sum +++ b/go.sum @@ -52,8 +52,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= -github.com/apache/cloudstack-go/v2 v2.19.0 h1:YHLw770MmgiqXx6NRFYw2Nr7DpnylLhLG2KYNCftgnc= -github.com/apache/cloudstack-go/v2 v2.19.0/go.mod h1:p/YBUwIEkQN6CQxFhw8Ff0wzf1MY0qRRRuGYNbcb1F8= +github.com/apache/cloudstack-go/v2 v2.19.1 h1:1K5O4NZpdWzOZUN6XuaVNsdX+QnoFRc5VE/oc1nUckQ= +github.com/apache/cloudstack-go/v2 v2.19.1/go.mod h1:p/YBUwIEkQN6CQxFhw8Ff0wzf1MY0qRRRuGYNbcb1F8= github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=