Add GHA to run MCS conformance tests

.github/workflows/e2e.yml

@@ -68,3 +68,49 @@ jobs:
       - name: Post mortem
         if: failure()
         uses: submariner-io/shipyard/gh-actions/post-mortem@devel
+  conformance-test:
+    name: MCS Conformance
+    needs: images
+    timeout-minutes: 60
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        using: ['clusterset-ip', '']
+    steps:
+      - name: Check out the repository
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
+
+      - name: Check out the mcs-api repository
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
+        with:
+          ref: 2b1ebf0d1a12261b9982393a5b0e4abceefa437f
+          repository: kubernetes-sigs/mcs-api
+          path: mcs-api
+
+      - name: Deploy Submariner
+        shell: bash
+        run: |
+          make deploy using="${{ matrix.using }}"
+
+      - name: Run conformance tests
+        shell: bash
+        run: |
+          export KUBECONFIG=$(find $(git rev-parse --show-toplevel)/output/kubeconfigs/ -type f -printf %p:)
+          label_filter="(!ClusterIP || Connectivity) && !ExportedLabels"
+          if [[ "${{ matrix.using }}" =~ "clusterset-ip" ]]; then
+                label_filter="ClusterIP && !Connectivity"
+          fi
+          cd mcs-api/conformance
+          go test -v -timeout 30m -contexts cluster1,cluster2 -args -test.timeout 15m \
+              --ginkgo.v --ginkgo.trace --ginkgo.label-filter "${label_filter}"
+
+      - name: Print report.html
+        if: always()
+        shell: bash
+        run: |
+          cat mcs-api/conformance/report.html
+
+      - name: Post mortem
+        if: failure()
+        uses: submariner-io/shipyard/gh-actions/post-mortem@devel

.golangci.yml

@@ -39,29 +39,40 @@ linters-settings:
 linters:
   disable-all: true
   enable:
+    - asasalint
     - asciicheck
     - bidichk
     - bodyclose
+    # - canonicalheader # This is a slow linter and we don't use the net/http.Header API
+    - containedctx
     - contextcheck
     - copyloopvar
     # - cyclop # This is equivalent to gocyclo
+    - decorder
     # - depguard # depguard now denies by default, it should only be enabled if we actually use it
     - dogsled
     - dupl
+    - dupword
     - durationcheck
     - errcheck
+    - errchkjson
     - errorlint
     - errname
+    # - execinquery # No SQ
     - exhaustive
-    # - exhaustivestruct # Not recommended for general use - meant to be used only for special cases
+    # - exhaustruct # This is too cumbersome as it requires all string, int, pointer et al fields to be initialized even when the
+    #                 type's default suffices, which is most of the time
+    - fatcontext
     # - forbidigo # We don't forbid any statements
     # - forcetypeassert # There are many unchecked type assertions that would be the result of a programming error so the
     #                     reasonable recourse would be to panic anyway if checked so this doesn't seem useful
     # - funlen # gocyclo is enabled which is generally a better metric than simply LOC.
     - gci
     - ginkgolinter
+    - gocheckcompilerdirectives
     # - gochecknoglobals # We don't want to forbid global variable constants
     # - gochecknoinits # We use init functions for valid reasons
+    # - gochecksumtype # The usefulness is very narrow
     - gocognit
     - goconst
     - gocritic
@@ -73,55 +84,72 @@ linters:
     - gofumpt
     - goheader
     - goimports
-    # - golint # Deprecated since v1.41.0
-    # - gomnd # It doesn't seem useful in general to enforce constants for all numeric values
     # - gomoddirectives # We don't want to forbid the 'replace' directive
     # - gomodguard # We don't block any modules
     # - goprintffuncname # This doesn't seem useful at all
+    # - gosmopolitan # This is related to internationalization which is not a concern for us
     - gosec
     - gosimple
     - govet
-    # - ifshort # This is a style preference and doesn't seem compelling
+    - grouper
+    - iface
     - importas
+    - inamedparam
     - ineffassign
+    # - interfacebloat # We track complexity elsewhere
+    - intrange
     # - ireturn # The argument to always "Return Concrete Types" doesn't seem compelling. It is perfectly valid to return
     #             an interface to avoid exposing the entire underlying struct
-    # - interfacer # Deprecated since v1.38.0
     - lll
+    - loggercheck
+    - maintidx
     - makezero
-    # - maligned # Deprecated since v1.38.0
+    - mirror
     - misspell
+    # - mnd # It doesn't seem useful in general to enforce constants for all numeric values
     - nakedret
     # - nestif # This calculates cognitive complexity but we're doing that elsewhere
     - nilerr
     - nilnil
     # - nlreturn # This is reasonable with a block-size of 2 but setting it above isn't honored
     # - noctx # We don't send HTTP requests
     - nolintlint
+    - nonamedreturns
+    # - nosprintfhostport # The use of this is very narrow
     # - paralleltest # Not relevant for Ginkgo UTs
+    - perfsprint
     - prealloc
     - predeclared
     - promlinter
+    - protogetter
+    - reassign
+    - recvcheck
     - revive
     # - rowserrcheck # We don't use SQL
-    # - scopelint # Deprecated since v1.39.0
+    # - sloglint # We don't use log/slog
+    # - spancheck # We don't use OpenTelemetry/OpenCensus
     # - sqlclosecheck # We don't use SQL
     - staticcheck
     - stylecheck
+    - tagalign
     # - tagliatelle # Inconsistent with stylecheck and not as good
     # - tenv # Not relevant for our Ginkgo UTs
+    # - testableexamples # We don't need this
+    # - testifylint # We don't use testify
     - testpackage
     # - thelper # Not relevant for our Ginkgo UTs
     # - tparallel # Not relevant for our Ginkgo UTs
     - typecheck
     - unconvert
     - unparam
     - unused
+    - usestdlibvars
     # - varnamelen # It doesn't seem necessary to enforce a minimum variable name length
     - wastedassign
     - whitespace
     - wrapcheck
     - wsl
+    # - zerologlint # We use zerolog indirectly so this isn't needed
 issues:
   exclude-rules:
     # Allow dot-imports for Gomega BDD directives per idiomatic Gomega

coredns/gateway/controller.go

@@ -20,7 +20,6 @@ package gateway
 
 import (
 	"context"
-	"fmt"
 	"os"
 	"sync/atomic"
 
@@ -110,7 +109,7 @@ func (c *Controller) Start(client dynamic.Interface) error {
 	go c.informer.Run(c.stopCh)
 
 	if ok := cache.WaitForCacheSync(c.stopCh, c.informer.HasSynced); !ok {
-		return fmt.Errorf("failed to wait for informer cache to sync")
+		return errors.New("failed to wait for informer cache to sync")
 	}
 
 	go c.queue.Run(c.stopCh, c.processNextGateway)
@@ -204,7 +203,7 @@ func (c *Controller) updateLocalClusterIDIfNeeded(clusterID string) {
 	}
 }
 
-func getGatewayStatus(obj *unstructured.Unstructured) (connections []interface{}, clusterID string, gwStatus bool) {
+func getGatewayStatus(obj *unstructured.Unstructured) ([]interface{}, string, bool) {
 	status, found, err := unstructured.NestedMap(obj.Object, "status")
 	if !found || err != nil {
 		logger.Errorf(err, "status field not found in %#v, err was", obj)
@@ -213,6 +212,8 @@ func getGatewayStatus(obj *unstructured.Unstructured) (connections []interface{}
 
 	localClusterID, found, err := unstructured.NestedString(status, "localEndpoint", "cluster_id")
 
+	var connections []interface{}
+
 	if !found || err != nil {
 		logger.Errorf(err, "localEndpoint->cluster_id not found in %#v, err was", status)
 

coredns/loadbalancer/smooth_weighted_round_robin.go

@@ -21,6 +21,7 @@ package loadbalancer
 import (
 	"fmt"
 
+	"github.com/pkg/errors"
 	"github.com/submariner-io/admiral/pkg/log"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
@@ -65,9 +66,9 @@ func (lb *smoothWeightedRR) ItemCount() int {
 }
 
 // Add - adds a new unique item to the list.
-func (lb *smoothWeightedRR) Add(item interface{}, weight int64) (err error) {
+func (lb *smoothWeightedRR) Add(item interface{}, weight int64) error {
 	if item == nil {
-		return fmt.Errorf("item cannot be nil")
+		return errors.New("item cannot be nil")
 	}
 
 	if weight < 0 {
@@ -115,7 +116,8 @@ func (lb *smoothWeightedRR) nextWeightedItem() *weightedItem {
 	return nextSmoothWeightedItem(lb.items)
 }
 
-func nextSmoothWeightedItem(items []*weightedItem) (best *weightedItem) {
+func nextSmoothWeightedItem(items []*weightedItem) *weightedItem {
+	var best *weightedItem
 	total := int64(0)
 
 	for _, item := range items {

coredns/loadbalancer/smooth_weighted_round_robin_test.go

@@ -59,7 +59,7 @@ var _ = Describe("Smooth Weighted RR", func() {
 
 	// Validations
 	validateServerAdded := func(s server) {
-		for i := 0; i < 100; i++ {
+		for range 100 {
 			if lb.Next() == s.name {
 				return
 			}
@@ -75,15 +75,15 @@ var _ = Describe("Smooth Weighted RR", func() {
 
 	validateEmptyLBState := func() {
 		Expect(lb.ItemCount()).To(Equal(0))
-		for i := 0; i < 100; i++ {
+		for range 100 {
 			Expect(lb.Next()).To(BeNil())
 		}
 	}
 
 	validateLoadBalancingByCount := func(rounds int, servers []server) {
 		totalServersWeight := getTotalServesWeight(servers)
 		results := make(map[string]int64)
-		for i := 0; i < rounds; i++ {
+		for range rounds {
 			s := lb.Next().(string)
 			results[s]++
 		}
@@ -170,7 +170,7 @@ var _ = Describe("Smooth Weighted RR", func() {
 		It("Next() should return it all the time", func() {
 			s := servers[0]
 			addServer(s)
-			for i := 0; i < 10; i++ {
+			for range 10 {
 				Expect(lb.Next()).To(Equal(s.name))
 			}
 		})
@@ -192,7 +192,7 @@ var _ = Describe("Smooth Weighted RR", func() {
 				err := lb.Add(s.name, s.weight)
 				Expect(err).ToNot(HaveOccurred())
 			}
-			for i := 0; i < 100; i++ {
+			for range 100 {
 				for _, s := range roundRobinServers {
 					Expect(lb.Next().(string)).To(Equal(s.name))
 				}

coredns/resolver/clusterip_service_test.go

@@ -56,15 +56,15 @@ func testClusterIPServiceInOneCluster() {
 
 	Context("and no specific cluster is requested", func() {
 		It("should consistently return its DNS record", func() {
-			for i := 0; i < 5; i++ {
+			for range 5 {
 				t.assertDNSRecordsFound(namespace1, service1, "", "", false, expDNSRecord)
 			}
 		})
 	})
 
 	Context("and the cluster is requested", func() {
 		It("should consistently return its DNS record", func() {
-			for i := 0; i < 5; i++ {
+			for range 5 {
 				t.assertDNSRecordsFound(namespace1, service1, clusterID1, "", false, expDNSRecord)
 			}
 		})
@@ -145,7 +145,7 @@ func testClusterIPServiceInTwoClusters() {
 		})
 
 		It("should consistently return its DNS record", func() {
-			for i := 0; i < 10; i++ {
+			for range 10 {
 				Expect(t.getNonHeadlessDNSRecord(namespace1, service1, "").IP).To(Equal(serviceIP1))
 			}
 		})
@@ -164,7 +164,7 @@ func testClusterIPServiceInTwoClusters() {
 
 		Context("and no specific cluster is requested", func() {
 			It("should consistently return the DNS record of the connected cluster", func() {
-				for i := 0; i < 10; i++ {
+				for range 10 {
 					t.assertDNSRecordsFound(namespace1, service1, "", "", false, expDNSRecord)
 				}
 			})
@@ -210,7 +210,7 @@ func testClusterIPServiceInTwoClusters() {
 
 		Context("and no specific cluster is requested", func() {
 			It("should consistently return the DNS record of the healthy cluster", func() {
-				for i := 0; i < 10; i++ {
+				for range 10 {
 					t.assertDNSRecordsFound(namespace1, service1, "", "", false, expDNSRecord)
 				}
 			})
@@ -245,7 +245,7 @@ func testClusterIPServiceInTwoClusters() {
 		})
 
 		It("should consistently return the DNS record of the remaining cluster", func() {
-			for i := 0; i < 10; i++ {
+			for range 10 {
 				t.assertDNSRecordsFound(namespace1, service1, "", "", false, expDNSRecord)
 			}
 		})
@@ -279,7 +279,7 @@ func testClusterIPServiceInThreeClusters() {
 		})
 
 		It("should consistently return the merged service ports", func() {
-			for i := 0; i < 10; i++ {
+			for range 10 {
 				Expect(t.getNonHeadlessDNSRecord(namespace1, service1, "").Ports).To(Equal([]mcsv1a1.ServicePort{port1}))
 			}
 		})
@@ -293,7 +293,7 @@ func testClusterIPServiceInThreeClusters() {
 		}
 
 		It("should consistently return its DNS record", func() {
-			for i := 0; i < 10; i++ {
+			for range 10 {
 				t.assertDNSRecordsFound(namespace1, service1, clusterID2, "", false, expDNSRecord)
 			}
 		})
@@ -320,7 +320,7 @@ func testClusterIPServiceInThreeClusters() {
 
 		Context("and subsequently healthy again", func() {
 			It("should consistently return the all DNS records round-robin", func() {
-				for i := 0; i < 5; i++ {
+				for range 5 {
 					Expect(t.getNonHeadlessDNSRecord(namespace1, service1, "").IP).To(Or(Equal(serviceIP1), Equal(serviceIP3)))
 				}
 

coredns/resolver/resolver.go

@@ -30,7 +30,7 @@ func New(clusterStatus ClusterStatus, client dynamic.Interface) *Interface {
 	}
 }
 
-func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) (records []DNSRecord, isHeadless, found bool) {
+func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) ([]DNSRecord, bool, bool) {
 	i.mutex.RLock()
 	defer i.mutex.RUnlock()
 
@@ -48,7 +48,7 @@ func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) (
 		return nil, false, found
 	}
 
-	records, found = i.getHeadlessRecords(serviceInfo, clusterID, hostname)
+	records, found := i.getHeadlessRecords(serviceInfo, clusterID, hostname)
 
 	return records, true, found
 }

coredns/resolver/resolver_suite_test.go

@@ -259,15 +259,15 @@ func (t *testDriver) testRoundRobin(ns, service string, serviceIPs ...string) {
 	ipsCount := len(serviceIPs)
 	rrIPs := make([]string, 0)
 
-	for i := 0; i < ipsCount; i++ {
+	for i := range ipsCount {
 		r := t.getNonHeadlessDNSRecord(ns, service, "")
 		rrIPs = append(rrIPs, r.IP)
 		slice := rrIPs[0:i]
 		Expect(slice).ToNot(ContainElement(r.IP))
 		Expect(serviceIPs).To(ContainElement(r.IP))
 	}
 
-	for i := 0; i < 5; i++ {
+	for range 5 {
 		for _, ip := range rrIPs {
 			testIP := t.getNonHeadlessDNSRecord(ns, service, "").IP
 			Expect(testIP).To(Equal(ip))