Skip to content

Commit

Permalink
always return lbnotfound from getNodeBalancerForService
Browse files Browse the repository at this point in the history
  • Loading branch information
luthermonson committed Jan 19, 2024
1 parent 5572272 commit aaab0ca
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 17 deletions.
21 changes: 9 additions & 12 deletions cloud/linode/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,13 @@ func newLoadbalancers(client Client, zone string) cloudprovider.LoadBalancer {
}

func (l *loadbalancers) getNodeBalancerForService(ctx context.Context, service *v1.Service) (*linodego.NodeBalancer, error) {
rawID, _ := getServiceAnnotation(service, annLinodeNodeBalancerID)
rawID := service.GetAnnotations()[annLinodeNodeBalancerID]
id, idErr := strconv.Atoi(rawID)
hasIDAnn := idErr == nil && id != 0

if hasIDAnn {
sentry.SetTag(ctx, "load_balancer_id", rawID)
nb, err := l.getNodeBalancerByID(ctx, service, id)
switch err.(type) {
case nil:
return nb, nil

case lbNotFoundError:
return nil, fmt.Errorf("%s annotation points to a NodeBalancer that does not exist: %s", annLinodeNodeBalancerID, err)

default:
return nil, err
}
return l.getNodeBalancerByID(ctx, service, id)
}
return l.getNodeBalancerByStatus(ctx, service)
}
Expand Down Expand Up @@ -199,6 +189,13 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri
nb, err = l.getNodeBalancerForService(ctx, service)
switch err.(type) {
case lbNotFoundError:
if service.GetAnnotations()[annLinodeNodeBalancerID] != "" {
// a load balancer annotation has been created so a NodeBalancer is coming, error out and retry later
klog.Infof("NodeBalancer created but not available yet, waiting...")
sentry.CaptureError(ctx, err)
return nil, err
}

if nb, err = l.buildLoadBalancerRequest(ctx, clusterName, service, nodes); err != nil {
sentry.CaptureError(ctx, err)
return nil, err
Expand Down
10 changes: 6 additions & 4 deletions cloud/linode/loadbalancers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1776,10 +1776,12 @@ func testGetNodeBalancerForServiceIDDoesNotExist(t *testing.T, client *linodego.
t.Fatal("expected getNodeBalancerForService to return an error")
}

expectedErr := fmt.Sprintf("%s annotation points to a NodeBalancer that does not exist: LoadBalancer (%s) not found for service (%s)",
annLinodeNodeBalancerID, bogusNodeBalancerID, getServiceNn(svc))

if err.Error() != expectedErr {
nbid, _ := strconv.Atoi(bogusNodeBalancerID)
expectedErr := lbNotFoundError{
serviceNn: getServiceNn(svc),
nodeBalancerID: nbid,
}
if err.Error() != expectedErr.Error() {
t.Errorf("expected error to be '%s' but got '%s'", expectedErr, err)
}
}
Expand Down
103 changes: 102 additions & 1 deletion e2e/test/ccm_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import (
"fmt"
"os/exec"
"strconv"
"time"

"github.com/linode/linodego"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/types"
core "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/watch"
)
Expand Down Expand Up @@ -90,6 +92,14 @@ var _ = Describe("e2e tests", func() {
Eventually(watcher.ResultChan()).Should(Receive(EnsuredService()))
}

ensureServiceWasDeleted := func() {
notfound := func() bool {
_, err := f.LoadBalancer.GetService()
return errors.IsNotFound(err)
}
Eventually(notfound).WithTimeout(10 * time.Second).Should(BeTrue())
}

createServiceWithSelector := func(selector map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) {
Expect(f.LoadBalancer.CreateService(selector, nil, ports, isSessionAffinityClientIP)).NotTo(HaveOccurred())
Eventually(f.LoadBalancer.GetServiceEndpoints).Should(Not(BeEmpty()))
Expand Down Expand Up @@ -942,7 +952,7 @@ var _ = Describe("e2e tests", func() {
By("Creating new NodeBalancer")
nbID := createNodeBalancer()

By("Waiting for currenct NodeBalancer to be ready")
By("Waiting for current NodeBalancer to be ready")
checkNodeBalancerID(framework.TestServerResourceName, nodeBalancerID)

By("Annotating service with new NodeBalancer ID")
Expand All @@ -957,6 +967,97 @@ var _ = Describe("e2e tests", func() {
})
})

Context("Deleted Service when NodeBalancer not present", func() {
var (
pods []string
labels map[string]string
annotations map[string]string
servicePorts []core.ServicePort

nodeBalancerID int
)

BeforeEach(func() {
pods = []string{"test-pod-1"}
ports := []core.ContainerPort{
{
Name: "http-1",
ContainerPort: 8080,
},
}
servicePorts = []core.ServicePort{
{
Name: "http-1",
Port: 80,
TargetPort: intstr.FromInt(8080),
Protocol: "TCP",
},
}

labels = map[string]string{
"app": "test-loadbalancer-with-nodebalancer-id",
}

By("Creating NodeBalancer")
nodeBalancerID = createNodeBalancer()

annotations = map[string]string{
annLinodeNodeBalancerID: strconv.Itoa(nodeBalancerID),
}

By("Creating Pod")
createPodWithLabel(pods, ports, framework.TestServerImage, labels, false)

By("Creating Service")
createServiceWithAnnotations(labels, annotations, servicePorts, false)
})

AfterEach(func() {
By("Deleting the Pods")
deletePods(pods)

err := root.Recycle()
Expect(err).NotTo(HaveOccurred())
})

It("should use the specified NodeBalancer", func() {
By("Checking the NodeBalancerID")
checkNodeBalancerID(framework.TestServerResourceName, nodeBalancerID)
})

It("should use the newly specified NodeBalancer ID", func() {
By("Creating new NodeBalancer")
nbID := createNodeBalancer()

By("Waiting for current NodeBalancer to be ready")
checkNodeBalancerID(framework.TestServerResourceName, nodeBalancerID)

By("Annotating service with new NodeBalancer ID")
annotations[annLinodeNodeBalancerID] = strconv.Itoa(nbID)
updateServiceWithAnnotations(labels, annotations, servicePorts, false)

By("Checking the NodeBalancer ID")
checkNodeBalancerID(framework.TestServerResourceName, nbID)

By("Checking old NodeBalancer was deleted")
checkNodeBalancerNotExists(nodeBalancerID)
})

It("should delete the service with no NodeBalancer present", func() {
By("Deleting the NodeBalancer")
deleteNodeBalancer(nodeBalancerID)

By("Checking old NodeBalancer was deleted")
checkNodeBalancerNotExists(nodeBalancerID)

By("Deleting the Service")
deleteService()

By("Checking if the service was deleted")
ensureServiceWasDeleted()
})
})

Context("With Preserve Annotation", func() {
var (
pods []string
Expand Down
4 changes: 4 additions & 0 deletions e2e/test/framework/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ func (i *lbInvocation) GetServiceWatcher() (watch.Interface, error) {
return watcher, nil
}

func (i *lbInvocation) GetService() (*core.Service, error) {
return i.kubeClient.CoreV1().Services(i.Namespace()).Get(context.TODO(), TestServerResourceName, metav1.GetOptions{})
}

func (i *lbInvocation) CreateService(selector, annotations map[string]string, ports []core.ServicePort, isSessionAffinityClientIP bool) error {
return i.createOrUpdateService(selector, annotations, ports, isSessionAffinityClientIP, true)
}
Expand Down

0 comments on commit aaab0ca

Please sign in to comment.