From 35daf685f91182c8b58ae9bb05ddba34134c370e Mon Sep 17 00:00:00 2001 From: MICHAL MALKA Date: Tue, 30 Jul 2024 15:48:45 +0300 Subject: [PATCH] fix linter errors on existing files Signed-off-by: MICHAL MALKA --- pkg/controlplane/control/manager.go | 12 +++--- pkg/controlplane/control/port.go | 64 +++++++++++++++++++---------- tests/e2e/k8s/suite.go | 14 +++---- tests/e2e/k8s/util/k8s_yaml.go | 17 -------- 4 files changed, 56 insertions(+), 51 deletions(-) diff --git a/pkg/controlplane/control/manager.go b/pkg/controlplane/control/manager.go index 887c4a175..2ff38972a 100644 --- a/pkg/controlplane/control/manager.go +++ b/pkg/controlplane/control/manager.go @@ -71,8 +71,8 @@ func (e exportServiceNotExistError) Error() string { } func (e exportServiceNotExistError) Is(target error) bool { - _, ok := target.(*exportServiceNotExistError) - return ok + var expServErr *exportServiceNotExistError + return errors.As(target, &expServErr) } type importServiceNotExistError struct { @@ -86,8 +86,8 @@ func (e importServiceNotExistError) Error() string { } func (e importServiceNotExistError) Is(target error) bool { - _, ok := target.(*importServiceNotExistError) - return ok + var impServErr *importServiceNotExistError + return errors.As(target, &impServErr) } type conflictingServiceError struct { @@ -102,8 +102,8 @@ func (e conflictingServiceError) Error() string { } func (e conflictingServiceError) Is(target error) bool { - _, ok := target.(*conflictingServiceError) - return ok + var confServErr *conflictingServiceError + return errors.As(target, &confServErr) } type importEndpointSliceName struct { diff --git a/pkg/controlplane/control/port.go b/pkg/controlplane/control/port.go index c075a6d51..829c7678a 100644 --- a/pkg/controlplane/control/port.go +++ b/pkg/controlplane/control/port.go @@ -14,6 +14,7 @@ package control import ( + "errors" "fmt" "math/rand" "sync" @@ -46,8 +47,8 @@ func (e conflictingTargetPortError) Error() string { } func (e conflictingTargetPortError) Is(target error) bool { - _, ok := target.(*conflictingTargetPortError) - return ok + var confPortErr *conflictingTargetPortError + return errors.As(target, &confPortErr) } // portManager leases ports for use by imported services. @@ -98,31 +99,52 @@ func (m *portManager) Lease(name types.NamespacedName, port uint16) (uint16, err defer m.lock.Unlock() if port == 0 { - if len(m.leasesByPort) == int(portCount) { - return 0, fmt.Errorf("all ports are taken") - } + return m.leaseWithRandomPort(name) + } + return m.leaseWithSpecificPort(name, port) +} - if port = m.leasesByName[name]; port != 0 { - m.logger.Infof("Leased existing: %d.", port) - } else { - port = m.getRandomFreePort() - m.logger.Infof("Generated port: %d.", port) - } - } else { - if leaseName, ok := m.leasesByPort[port]; ok && leaseName != name { - return 0, conflictingTargetPortError{ - port: port, - leaseName: leaseName, - } +// Lease random port. +func (m *portManager) leaseWithRandomPort(name types.NamespacedName) (uint16, error) { + if len(m.leasesByPort) == int(portCount) { + return 0, fmt.Errorf("all ports are taken") + } + + if port := m.leasesByName[name]; port != 0 { + m.logger.Infof("Leased existing: %d.", port) + return port, nil + } + + port := m.getRandomFreePort() + m.logger.Infof("Generated port: %d.", port) + + // Mark previous port (if exists) as free + if oldPort, ok := m.leasesByName[name]; ok { + delete(m.leasesByPort, oldPort) + } + + // Mark port as leased + m.leasesByPort[port] = name + m.leasesByName[name] = port + + return port, nil +} + +// Lease specific port. +func (m *portManager) leaseWithSpecificPort(name types.NamespacedName, port uint16) (uint16, error) { + if leaseName, ok := m.leasesByPort[port]; ok && leaseName != name { + return 0, conflictingTargetPortError{ + port: port, + leaseName: leaseName, } } - // mark previous port (if exists) is free - if port, ok := m.leasesByName[name]; ok { - delete(m.leasesByPort, port) + // Mark previous port (if exists) as free + if oldPort, ok := m.leasesByName[name]; ok { + delete(m.leasesByPort, oldPort) } - // mark port is leased + // Mark port as leased m.leasesByPort[port] = name m.leasesByName[name] = port diff --git a/tests/e2e/k8s/suite.go b/tests/e2e/k8s/suite.go index bdbcdb896..b2179fc29 100644 --- a/tests/e2e/k8s/suite.go +++ b/tests/e2e/k8s/suite.go @@ -88,19 +88,19 @@ func (s *TestSuite) SetupSuite() { } // wait for clusters - for i := 0; i < clusterCount; i++ { - if err := s.clusters[i].Wait(); err != nil { + for clusterNum := 0; clusterNum < clusterCount; clusterNum++ { + if err := s.clusters[clusterNum].Wait(); err != nil { s.T().Fatal(err) } // create http-echo service which echoes the cluster name - err := s.clusters[i].CreatePodAndService(httpecho.ServerPod(httpEchoService, s.clusters[i].Name())) + err := s.clusters[clusterNum].CreatePodAndService(httpecho.ServerPod(httpEchoService, s.clusters[clusterNum].Name())) if err != nil { s.T().Fatal(fmt.Errorf("cannot create http-echo service: %w", err)) } // create iperf3 server service - err = s.clusters[i].CreatePodAndService(iperf3.ServerPod(iperf3Service)) + err = s.clusters[clusterNum].CreatePodAndService(iperf3.ServerPod(iperf3Service)) if err != nil { s.T().Fatal(fmt.Errorf("cannot create iperf3-server service: %w", err)) } @@ -114,16 +114,16 @@ func (s *TestSuite) SetupSuite() { localImage := "cl-operator:latest" remoteImage := path.Join(config.DefaultRegistry, "cl-operator:latest") managerModified := strings.ReplaceAll(string(managerFile), remoteImage, localImage) - if err := s.clusters[i].CreateFromYAML(managerModified, controller.OperatorNamespace); err != nil { + if err := s.clusters[clusterNum].CreateFromYAML(managerModified, controller.OperatorNamespace); err != nil { s.T().Fatal(fmt.Errorf("cannot create k8s operator deployment files: %w", err)) } - if err := s.clusters[i].CreateFromPath("./../../../config/operator/rbac/"); err != nil { + if err := s.clusters[clusterNum].CreateFromPath("./../../../config/operator/rbac/"); err != nil { s.T().Fatal(fmt.Errorf("cannot create k8s operator rbac files: %w", err)) } // create the project CRDs. - if err := s.clusters[i].CreateFromPath("./../../../config/crds/"); err != nil { + if err := s.clusters[clusterNum].CreateFromPath("./../../../config/crds/"); err != nil { s.T().Fatal(fmt.Errorf("cannot create project CRDs: %w", err)) } } diff --git a/tests/e2e/k8s/util/k8s_yaml.go b/tests/e2e/k8s/util/k8s_yaml.go index af81d6a19..4072ca34b 100644 --- a/tests/e2e/k8s/util/k8s_yaml.go +++ b/tests/e2e/k8s/util/k8s_yaml.go @@ -32,23 +32,6 @@ func replaceOnce(s, search, replace string) (string, error) { return strings.ReplaceAll(s, search, replace), nil } -// remove removes a substring starting with until (excluding). -func remove(s, from, to string) (string, error) { - searchCount := strings.Count(s, from) - if searchCount != 1 { - return "", fmt.Errorf("found %d (!=1) occurrences of '%s'", searchCount, from) - } - - startPos := strings.Index(s, from) - tmpPos := strings.Index(s[startPos+len(from):], to) - if tmpPos == -1 { - return "", fmt.Errorf("cannot found termination for '%s'", from) - } - endPos := startPos + len(from) + tmpPos - - return s[:startPos] + s[endPos:], nil -} - func (f *Fabric) generateK8SYAML(p *peer, cfg *PeerConfig) (string, error) { logLevel := "info" if os.Getenv("DEBUG") == "1" {