Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up the coredns module (which wasn’t linted) #1505

Merged
merged 13 commits into from
Feb 15, 2024
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,5 @@ issues:
linters:
- gochecknoinits
- goerr113
- unparam
- wrapcheck
4 changes: 3 additions & 1 deletion coredns/gateway/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (c *Controller) Stop() {
logger.Infof("Gateway status Controller stopped")
}

func (c *Controller) processNextGateway(key, name, ns string) (bool, error) {
func (c *Controller) processNextGateway(key, _, _ string) (bool, error) {
obj, exists, err := c.store.GetByKey(key)
if err != nil {
// requeue the item to work on later.
Expand Down Expand Up @@ -181,6 +181,7 @@ func (c *Controller) updateClusterStatusMap(connections []interface{}) {
if newMap == nil {
newMap = copyMap(currentMap)
}

delete(newMap, clusterID)
}
}
Expand Down Expand Up @@ -250,6 +251,7 @@ func (c *Controller) getCheckedClientset(client dynamic.Interface) (dynamic.Reso
// First check if the Submariner resource is present.
gvr, _ := schema.ParseResourceArg("submariners.v1alpha1.submariner.io")
list, err := client.Resource(*gvr).Namespace(v1.NamespaceAll).List(context.TODO(), metav1.ListOptions{})

if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) || (err == nil && len(list.Items) == 0) {
return nil, apierrors.NewNotFound(gvr.GroupResource(), "")
}
Expand Down
17 changes: 9 additions & 8 deletions coredns/loadbalancer/smooth_weighted_round_robin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ type server struct {
weight int64
}

//nolint:gosec // This is only used to shuffle the list of resolvers
var randgen = rand.New(rand.NewSource(time.Now().UnixNano()))

var _ = Describe("Smooth Weighted RR", func() {
// Global Vars
var servers []server
Expand Down Expand Up @@ -123,20 +126,18 @@ var _ = Describe("Smooth Weighted RR", func() {
{name: "server2", weight: 1},
{name: "server3", weight: 1},
}
rand.Seed(time.Now().UnixNano())
servers = []server{
{name: "server1", weight: randInt()},
{name: "server2", weight: randInt()},
{name: "server3", weight: randInt()},
}

roundRobinServers = []server{
{name: "server1", weight: 1},
{name: "server2", weight: 1},
{name: "server3", weight: 1},
}

rand.Shuffle(len(servers), func(i, j int) { servers[i], servers[j] = servers[j], servers[i] })
randgen.Shuffle(len(servers), func(i, j int) { servers[i], servers[j] = servers[j], servers[i] })
})

When("first created", func() {
Expand Down Expand Up @@ -165,15 +166,15 @@ var _ = Describe("Smooth Weighted RR", func() {
When("a nil is added", func() {
It("should return an error", func() {
err := lb.Add(nil, 100)
Expect(err).ToNot(BeNil())
Expect(err).To(HaveOccurred())
validateEmptyLBState()
})
})

When("an item is added with a negative weight", func() {
It("should return an error", func() {
err := lb.Add(servers[0], -100)
Expect(err).ToNot(BeNil())
Expect(err).To(HaveOccurred())
validateEmptyLBState()
})
})
Expand All @@ -193,7 +194,7 @@ var _ = Describe("Smooth Weighted RR", func() {
s := servers[0]
addServer(s)
err := lb.Add(s.name, s.weight)
Expect(err).ToNot(BeNil())
Expect(err).To(HaveOccurred())
Expect(lb.ItemCount()).To(Equal(1))
})
})
Expand All @@ -202,7 +203,7 @@ var _ = Describe("Smooth Weighted RR", func() {
It("should balance between them in an equal manner", func() {
for _, s := range roundRobinServers {
err := lb.Add(s.name, s.weight)
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
}
for i := 0; i < 100; i++ {
for _, s := range roundRobinServers {
Expand Down Expand Up @@ -253,7 +254,7 @@ var _ = Describe("Smooth Weighted RR", func() {
newServer := server{name: "server4", weight: 1}
smoothTestingServers = append(smoothTestingServers, newServer)
err := lb.Add(newServer.name, newServer.weight)
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())

Expect(lb.Next().(string)).To(Equal(smoothTestingServers[0].name))
Expect(lb.Next().(string)).To(Equal(smoothTestingServers[2].name))
Expand Down
1 change: 1 addition & 0 deletions coredns/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ var (

func init() {
flag.BoolVar(&showVersion, "subm-version", showVersion, "Show version")

dnsserver.Directives = directives
}

Expand Down
9 changes: 4 additions & 5 deletions coredns/plugin/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ type FailingResponseWriter struct {
errorMsg string
}

func (w *FailingResponseWriter) WriteMsg(m *dns.Msg) error {
func (w *FailingResponseWriter) WriteMsg(_ *dns.Msg) error {
return errors.New(w.errorMsg)
}

Expand Down Expand Up @@ -317,6 +317,7 @@ func testWithFallback() {
m := new(dns.Msg)
m.SetRcode(r, dns.RcodeBadCookie)
_ = w.WriteMsg(m)

return dns.RcodeBadCookie, nil
})

Expand Down Expand Up @@ -991,7 +992,6 @@ func (t *handlerTestDriver) executeTestCase(rec *dnstest.Recorder, tc test.Case)
}
}

//nolint:unparam // `name` always receives `service1'.
func newServiceImport(namespace, name string, siType mcsv1a1.ServiceImportType) *mcsv1a1.ServiceImport {
return &mcsv1a1.ServiceImport{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -1004,9 +1004,8 @@ func newServiceImport(namespace, name string, siType mcsv1a1.ServiceImportType)
}
}

//nolint:unparam // `namespace` always receives `namespace1`.
func newEndpointSlice(namespace, name, clusterID string, ports []mcsv1a1.ServicePort,
endpoints ...discovery.Endpoint) *discovery.EndpointSlice {
func newEndpointSlice(namespace, name, clusterID string, ports []mcsv1a1.ServicePort, endpoints ...discovery.Endpoint,
) *discovery.EndpointSlice {
epPorts := make([]discovery.EndpointPort, len(ports))
for i := range ports {
epPorts[i] = discovery.EndpointPort{
Expand Down
2 changes: 2 additions & 0 deletions coredns/plugin/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ func (lh *Lighthouse) createSRVRecords(dnsrecords []resolver.DNSRecord, state *r
reqPorts = dnsRecord.Ports
} else {
log.Debugf("Requested port %q, protocol %q for SRV", pReq.port, pReq.protocol)

for _, port := range dnsRecord.Ports {
name := strings.ToLower(port.Name)
protocol := strings.ToLower(string(port.Protocol))

log.Debugf("Checking port %q, protocol %q", name, protocol)

if name == pReq.port && protocol == pReq.protocol {
reqPorts = append(reqPorts, port)
}
Expand Down
10 changes: 7 additions & 3 deletions coredns/plugin/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ var (
buildKubeConfigFunc = clientcmd.BuildConfigFromFlags

newDynamicClient = func(c *rest.Config) (dynamic.Interface, error) {
return dynamic.NewForConfig(c)
client, err := dynamic.NewForConfig(c)
return client, errors.Wrap(err, "error creating a dynamic client")
}

restMapper meta.RESTMapper
Expand Down Expand Up @@ -109,7 +110,7 @@ func lighthouseParse(c *caddy.Controller) (*Lighthouse, error) {

resolverController := resolver.NewController(lh.Resolver)

err = resolverController.Start(watcher.Config{
err = resolverController.Start(&watcher.Config{
RestConfig: cfg,
Client: localClient,
RestMapper: restMapper,
Expand All @@ -121,6 +122,7 @@ func lighthouseParse(c *caddy.Controller) (*Lighthouse, error) {
c.OnShutdown(func() error {
gwController.Stop()
resolverController.Stop()

return nil
})

Expand All @@ -136,8 +138,10 @@ func lighthouseParse(c *caddy.Controller) (*Lighthouse, error) {
for i, str := range lh.Zones {
hosts := plugin.Host(str).NormalizeExact()
if hosts == nil {
c.Errf("Failed to normalize zone %q", str)
log.Infof("Failed to normalize zone %q", str)

lh.Zones[i] = ""

continue
}

Expand Down
3 changes: 2 additions & 1 deletion coredns/plugin/setup_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import (

type fakeHandler struct{}

func (f *fakeHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
func (f *fakeHandler) ServeDNS(_ context.Context, _ dns.ResponseWriter, _ *dns.Msg) (int, error) {
return dns.RcodeSuccess, nil
}

Expand Down Expand Up @@ -151,6 +151,7 @@ func testCorrectConfig() {
c := caddy.NewTestController("dns", config)
lh, err := lighthouseParse(c)
Expect(err).NotTo(HaveOccurred())

setupErr := setupLighthouse(c) // For coverage
Expect(setupErr).NotTo(HaveOccurred())
Expect(lh.Fall).Should(Equal(fall.F{}))
Expand Down
10 changes: 5 additions & 5 deletions coredns/resolver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
discovery "k8s.io/api/discovery/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
k8slabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
mcsv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
Expand All @@ -42,14 +41,14 @@ type controller struct {
stopCh chan struct{}
}

func NewController(r *Interface) *controller {
func NewController(r *Interface) *controller { //nolint:revive // Returning a private type doesn't cause problems here.
return &controller{
resolver: r,
stopCh: make(chan struct{}),
}
}

func (c *controller) Start(config watcher.Config) error {
func (c *controller) Start(config *watcher.Config) error {
logger.Infof("Starting Resolver Controller")

config.ResourceConfigs = []watcher.ResourceConfig{
Expand Down Expand Up @@ -78,7 +77,7 @@ func (c *controller) Start(config watcher.Config) error {

var err error

c.resourceWatcher, err = watcher.New(&config)
c.resourceWatcher, err = watcher.New(config)
if err != nil {
return errors.Wrap(err, "error creating the resource watcher")
}
Expand Down Expand Up @@ -111,13 +110,14 @@ func (c *controller) onEndpointSliceCreateOrUpdate(obj runtime.Object, _ int) bo
}

func (c *controller) getAllEndpointSlices(forEPS *discovery.EndpointSlice) []*discovery.EndpointSlice {
list := c.resourceWatcher.ListResources(&discovery.EndpointSlice{}, k8slabels.SelectorFromSet(map[string]string{
list := c.resourceWatcher.ListResources(&discovery.EndpointSlice{}, labels.SelectorFromSet(map[string]string{
constants.LabelSourceNamespace: forEPS.Labels[constants.LabelSourceNamespace],
mcsv1a1.LabelServiceName: forEPS.Labels[mcsv1a1.LabelServiceName],
constants.MCSLabelSourceCluster: forEPS.Labels[constants.MCSLabelSourceCluster],
}))

var epSlices []*discovery.EndpointSlice

for i := range list {
eps := list[i].(*discovery.EndpointSlice)
if !isOnBroker(eps) && !isLegacyEndpointSlice(eps) {
Expand Down
3 changes: 1 addition & 2 deletions coredns/resolver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ package resolver_test
import (
"context"

"github.com/submariner-io/lighthouse/coredns/constants"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/submariner-io/admiral/pkg/syncer/test"
"github.com/submariner-io/lighthouse/coredns/constants"
"github.com/submariner-io/lighthouse/coredns/resolver"
discovery "k8s.io/api/discovery/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down
16 changes: 9 additions & 7 deletions coredns/resolver/endpoint_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ import (
"strconv"
"strings"

utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/pkg/errors"
"github.com/submariner-io/admiral/pkg/resource"
"github.com/submariner-io/lighthouse/coredns/constants"
discovery "k8s.io/api/discovery/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/sets"
mcsv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
)

Expand Down Expand Up @@ -95,7 +95,8 @@ func (i *Interface) PutEndpointSlices(endpointSlices ...*discovery.EndpointSlice
return false
}

func (i *Interface) putClusterIPEndpointSlice(key, clusterID string, endpointSlice *discovery.EndpointSlice, serviceInfo *serviceInfo) bool {
func (i *Interface) putClusterIPEndpointSlice(key, clusterID string, endpointSlice *discovery.EndpointSlice, serviceInfo *serviceInfo,
) bool {
_, found := endpointSlice.Labels[constants.LabelIsHeadless]
if !found {
// This is a legacy pre-0.15 EndpointSlice.
Expand Down Expand Up @@ -167,8 +168,8 @@ func (i *Interface) putHeadlessEndpointSlices(key, clusterID string, endpointSli
switch {
case endpoint.Hostname != nil && *endpoint.Hostname != "":
hostname = *endpoint.Hostname
case endpoint.TargetRef != nil && strings.ToLower((*endpoint.TargetRef).Kind) == "pod":
hostname = (*endpoint.TargetRef).Name
case endpoint.TargetRef != nil && strings.EqualFold(endpoint.TargetRef.Kind, "pod"):
hostname = endpoint.TargetRef.Name
}

for _, address := range endpoint.Addresses {
Expand Down Expand Up @@ -220,7 +221,7 @@ func (i *Interface) getLocalEndpointSlices(forEPS *discovery.EndpointSlice) ([]*
}).String(),
})
if err != nil {
return nil, err
return nil, errors.Wrapf(err, "error retrieving the endpointslices in namespace %s", forEPS.Labels[constants.LabelSourceNamespace])
}

if len(list.Items) == 0 {
Expand All @@ -229,6 +230,7 @@ func (i *Interface) getLocalEndpointSlices(forEPS *discovery.EndpointSlice) ([]*
}

epSlices := make([]*discovery.EndpointSlice, len(list.Items))

for i := range list.Items {
epSlice := &discovery.EndpointSlice{}
err := runtime.DefaultUnstructuredConverter.FromUnstructured(list.Items[i].Object, epSlice)
Expand Down
1 change: 1 addition & 0 deletions coredns/resolver/headless_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func testHeadlessService() {

BeforeEach(func() {
t.resolver.PutServiceImport(newHeadlessAggregatedServiceImport(namespace1, service1))

annotations = nil
endpointSlice = nil
})
Expand Down
3 changes: 2 additions & 1 deletion coredns/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func New(clusterStatus ClusterStatus, client dynamic.Interface) *Interface {
}
}

func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) (records []DNSRecord, isHeadless bool, found bool) {
func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) (records []DNSRecord, isHeadless, found bool) {
i.mutex.RLock()
defer i.mutex.RUnlock()

Expand All @@ -49,6 +49,7 @@ func (i *Interface) GetDNSRecords(namespace, name, clusterID, hostname string) (
}

records, found = i.getHeadlessRecords(serviceInfo, clusterID, hostname)

return records, true, found
}

Expand Down
2 changes: 1 addition & 1 deletion coredns/resolver/resolver_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func newTestDriver() *testDriver {
t.resolver = resolver.New(t.clusterStatus, client)
controller := resolver.NewController(t.resolver)

Expect(controller.Start(watcher.Config{
Expect(controller.Start(&watcher.Config{
RestMapper: restMapper,
Client: client,
})).To(Succeed())
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/discovery/headless_services.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func RunHeadlessDiscoveryClusterNameTest(f *lhframework.Framework) {
clusterBName, false, true, true)
}

//nolint:gocognit,unparam // This really isn't that complex and would be awkward to refactor.
//nolint:gocognit // This really isn't that complex and would be awkward to refactor.
func verifyHeadlessSRVRecordsWithDig(f *framework.Framework, cluster framework.ClusterIndex, service *corev1.Service,
targetPod *corev1.PodList, hostNameList, domains []string, clusterName string, withPort, withcluster, shouldContain bool,
) {
Expand Down
1 change: 0 additions & 1 deletion test/e2e/discovery/service_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,6 @@ func RunServicesClusterAvailabilityMultiClusterTest(f *lhframework.Framework) {
checkedDomains, "", false)
}

//nolint:unparam // Keep srcCluster as a parameter for consistency and possible future extensions
func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex, service *corev1.Service, targetPod *corev1.PodList,
domains []string, clusterName string, withPort, shouldContain bool,
) {
Expand Down
Loading
Loading