smi/test: add test coverage for pkg/smi

The SMI client has a lot of code but no tests.
`github.com/openservicemesh/osm/pkg/smi  coverage: 0.0% of statements`

This change uses fake kubernetes clients to test all the functions that
implement the MeshSpec interface. It also removes dead code.
It helps improve the code coverage from 0.0% to 74.6%.
`github.com/openservicemesh/osm/pkg/smi coverage: 74.6% of statements`.

Due to some limitations of fake kubernetes clients such as not being
able to enforce label selector semantics with client informers and
caches, a few code paths repeated in each function are not testable.

Part of openservicemesh#1489

Signed-off-by: Shashank Ram <shashank08@gmail.com>

DESIGN.md

@@ -425,37 +425,32 @@ The `MeshSpec` implementation **has no awareness** of:
 ```go
 // MeshSpec is an interface declaring functions, which provide the specs for a service mesh declared with SMI.
 type MeshSpec interface {
-	// ListTrafficSplits lists TrafficSplit SMI resources.
+	// ListTrafficSplits lists SMI TrafficSplit resources
 	ListTrafficSplits() []*split.TrafficSplit
 
-	// ListTrafficSplitServices fetches all services declared with SMI Spec.
+	// ListTrafficSplitServices lists WeightedServices for the services specified in TrafficSplit SMI resources
 	ListTrafficSplitServices() []service.WeightedService
 
-	// ListServiceAccounts fetches all service accounts declared with SMI Spec.
+	// ListServiceAccounts lists ServiceAccount resources specified in SMI TrafficTarget resources
 	ListServiceAccounts() []service.K8sServiceAccount
 
-	// GetService fetches a specific service declared in SMI.
+	// GetService fetches a Kubernetes Service resource for the given MeshService
 	GetService(service.MeshService) *corev1.Service
 
-	// ListHTTPTrafficSpecs lists TrafficSpec SMI resources.
+	// ListServices Lists Kubernets Service resources that are part of monitored namespaces
+	ListServices() []*corev1.Service
+
+	// ListHTTPTrafficSpecs lists SMI HTTPRouteGroup resources
 	ListHTTPTrafficSpecs() []*spec.HTTPRouteGroup
 
-	// ListTrafficTargets lists TrafficTarget SMI resources.
+	// ListTrafficTargets lists SMI TrafficTarget resources
 	ListTrafficTargets() []*target.TrafficTarget
 
-	// ListBackpressures lists Backpressure resources.
-	// This is an experimental feature, which will eventually
-	// in some shape or form make its way into SMI Spec.
-	ListBackpressures() []*backpressure.Backpressure
-
-	// GetBackpressurePolicy gets the Backpressure policy corresponding to the MeshService
+	// GetBackpressurePolicy fetches the Backpressure policy for the MeshService
 	GetBackpressurePolicy(service.MeshService) *backpressure.Backpressure
 
-	// GetAnnouncementsChannel returns the channel on which SMI makes announcements
+	// GetAnnouncementsChannel returns the channel on which SMI client makes announcements
 	GetAnnouncementsChannel() <-chan interface{}
-
-	// ListServices returns a list of services that are part of monitored namespaces
-	ListServices() []*corev1.Service
 }
 ```
 

go.sum

@@ -1230,6 +1230,7 @@ k8s.io/cli-runtime v0.18.0 h1:jG8XpSqQ5TrV0N+EZ3PFz6+gqlCk71dkggWCCq9Mq34=
 k8s.io/cli-runtime v0.18.0/go.mod h1:1eXfmBsIJosjn9LjEBUd2WVPoPAY9XGTqTFcPMIBsUQ=
 k8s.io/client-go v0.18.0 h1:yqKw4cTUQraZK3fcVCMeSa+lqKwcjZ5wtcOIPnxQno4=
 k8s.io/client-go v0.18.0/go.mod h1:uQSYDYs4WhVZ9i6AIoEZuwUggLVEF64HOD37boKAtF8=
+k8s.io/client-go v11.0.0+incompatible h1:LBbX2+lOwY9flffWlJM7f1Ct8V2SRNiMRDFeiwnJo9o=
 k8s.io/code-generator v0.18.0/go.mod h1:+UHX5rSbxmR8kzS+FAv7um6dtYrZokQvjHpDSYRVkTc=
 k8s.io/component-base v0.18.0 h1:I+lP0fNfsEdTDpHaL61bCAqTZLoiWjEEP304Mo5ZQgE=
 k8s.io/component-base v0.18.0/go.mod h1:u3BCg0z1uskkzrnAKFzulmYaEpZF7XC9Pf/uFyb1v2c=

pkg/smi/client.go

@@ -43,7 +43,7 @@ func NewMeshSpecClient(smiKubeConfig *rest.Config, kubeClient kubernetes.Interfa
 		backpressureClientSet = backpressureClient.NewForConfigOrDie(smiKubeConfig)
 	}
 
-	client := newSMIClient(
+	client, err := newSMIClient(
 		kubeClient,
 		smiTrafficSplitClientSet,
 		smiTrafficSpecClientSet,
@@ -52,13 +52,10 @@ func NewMeshSpecClient(smiKubeConfig *rest.Config, kubeClient kubernetes.Interfa
 		osmNamespace,
 		namespaceController,
 		kubernetesClientName,
+		stop,
 	)
 
-	err := client.run(stop)
-	if err != nil {
-		return client, errors.Errorf("Could not start %s client", kubernetesClientName)
-	}
-	return client, nil
+	return client, err
 }
 
 func (c *Client) run(stop <-chan struct{}) error {
@@ -104,18 +101,13 @@ func (c *Client) run(stop <-chan struct{}) error {
 	return nil
 }
 
-// GetID implements endpoints.Provider interface and returns a string descriptor / identifier of the compute provider.
-func (c *Client) GetID() string {
-	return c.providerIdent
-}
-
 // GetAnnouncementsChannel returns the announcement channel for the SMI client.
 func (c *Client) GetAnnouncementsChannel() <-chan interface{} {
 	return c.announcements
 }
 
 // newClient creates a provider based on a Kubernetes client instance.
-func newSMIClient(kubeClient kubernetes.Interface, smiTrafficSplitClient *smiTrafficSplitClient.Clientset, smiTrafficSpecClient *smiTrafficSpecClient.Clientset, smiTrafficTargetClient *smiTrafficTargetClient.Clientset, backpressureClient *backpressureClient.Clientset, osmNamespace string, namespaceController namespace.Controller, providerIdent string) *Client {
+func newSMIClient(kubeClient kubernetes.Interface, smiTrafficSplitClient smiTrafficSplitClient.Interface, smiTrafficSpecClient smiTrafficSpecClient.Interface, smiTrafficTargetClient smiTrafficTargetClient.Interface, backpressureClient backpressureClient.Interface, osmNamespace string, namespaceController namespace.Controller, providerIdent string, stop chan struct{}) (*Client, error) {
 	informerFactory := informers.NewSharedInformerFactory(kubeClient, k8s.DefaultKubeEventResyncInterval)
 	smiTrafficSplitInformerFactory := smiTrafficSplitInformers.NewSharedInformerFactory(smiTrafficSplitClient, k8s.DefaultKubeEventResyncInterval)
 	smiTrafficSpecInformerFactory := smiTrafficSpecInformers.NewSharedInformerFactory(smiTrafficSpecClient, k8s.DefaultKubeEventResyncInterval)
@@ -164,7 +156,12 @@ func newSMIClient(kubeClient kubernetes.Interface, smiTrafficSplitClient *smiTra
 		informerCollection.Backpressure.AddEventHandler(k8s.GetKubernetesEventHandlers("Backpressure", "SMI", client.announcements, shouldObserve))
 	}
 
-	return &client
+	err := client.run(stop)
+	if err != nil {
+		return &client, errors.Errorf("Could not start %s client", kubernetesClientName)
+	}
+
+	return &client, err
 }
 
 // ListTrafficSplits implements mesh.MeshSpec by returning the list of traffic splits.
@@ -181,7 +178,7 @@ func (c *Client) ListTrafficSplits() []*split.TrafficSplit {
 	return trafficSplits
 }
 
-// ListHTTPTrafficSpecs implements mesh.Topology by returning the list of traffic specs.
+// ListHTTPTrafficSpecs lists SMI HTTPRouteGroup resources
 func (c *Client) ListHTTPTrafficSpecs() []*spec.HTTPRouteGroup {
 	var httpTrafficSpec []*spec.HTTPRouteGroup
 	for _, specIface := range c.caches.TrafficSpec.List() {
@@ -209,27 +206,6 @@ func (c *Client) ListTrafficTargets() []*target.TrafficTarget {
 	return trafficTargets
 }
 
-// ListBackpressures implements smi.MeshSpec and returns a list of backpressure policies.
-func (c *Client) ListBackpressures() []*backpressure.Backpressure {
-	var backpressureList []*backpressure.Backpressure
-
-	if !featureflags.IsBackpressureEnabled() {
-		log.Info().Msgf("Backpressure turned off!")
-		return backpressureList
-	}
-
-	for _, pressureIface := range c.caches.Backpressure.List() {
-		bpressure := pressureIface.(*backpressure.Backpressure)
-
-		if !c.namespaceController.IsMonitoredNamespace(bpressure.Namespace) {
-			continue
-		}
-		backpressureList = append(backpressureList, bpressure)
-	}
-
-	return backpressureList
-}
-
 // GetBackpressurePolicy gets the Backpressure policy corresponding to the MeshService
 func (c *Client) GetBackpressurePolicy(svc service.MeshService) *backpressure.Backpressure {
 	if !featureflags.IsBackpressureEnabled() {
@@ -278,7 +254,7 @@ func (c *Client) ListTrafficSplitServices() []service.WeightedService {
 	return services
 }
 
-// ListServiceAccounts implements mesh.MeshSpec by returning the service accounts observed from the given compute provider
+// ListServiceAccounts lists ServiceAccounts specified in SMI TrafficTarget resources
 func (c *Client) ListServiceAccounts() []service.K8sServiceAccount {
 	var serviceAccounts []service.K8sServiceAccount
 	for _, targetIface := range c.caches.TrafficTarget.List() {