From 14d296f5ddc4bdd7dde46ca2a12b3c39acadcab2 Mon Sep 17 00:00:00 2001
From: Rafal Augustyniak <raugustyniak@lyft.com>
Date: Fri, 23 Apr 2021 15:33:10 -0700
Subject: [PATCH] clean up in server plugins

---
 .../server_experimentation.go                 |   4 +-
 .../xds/ecds_faults_generator.go              |  21 +--
 .../xds/ecds_faults_generator_test.go         | 160 +++++++++---------
 .../xds/rtds_faults_generator.go              |  24 +--
 .../xds/rtds_faults_generator_test.go         |   4 +-
 5 files changed, 91 insertions(+), 122 deletions(-)

diff --git a/backend/module/chaos/serverexperimentation/server_experimentation.go b/backend/module/chaos/serverexperimentation/server_experimentation.go
index faa91c51..2ea8f81b 100644
--- a/backend/module/chaos/serverexperimentation/server_experimentation.go
+++ b/backend/module/chaos/serverexperimentation/server_experimentation.go
@@ -43,12 +43,12 @@ func New(untypedConfig *any.Any, logger *zap.Logger, scope tally.Scope) (module.
 		return nil, errors.New("service was not the correct type")
 	}
 
-	xds.RTDSGeneratorsByTypeUrl[xds.TypeUrl(&serverexperimentationv1.HTTPFaultConfig{})] = &serverexperimentationxds.RTDSServerFaultsResourceGenerator{
+	xds.RTDSGeneratorsByTypeUrl[xds.TypeUrl(&serverexperimentationv1.HTTPFaultConfig{})] = &serverexperimentationxds.RTDSFaultsGenerator{
 		IngressFaultRuntimePrefix: config.IngressFaultRuntimePrefix,
 		EgressFaultRuntimePrefix:  config.EgressFaultRuntimePrefix,
 	}
 
-	xds.ECDSGeneratorsByTypeUrl[xds.TypeUrl(&serverexperimentationv1.HTTPFaultConfig{})] = &serverexperimentationxds.ECDSServerFaultsResourceGenerator{}
+	xds.ECDSGeneratorsByTypeUrl[xds.TypeUrl(&serverexperimentationv1.HTTPFaultConfig{})] = &serverexperimentationxds.ECDSFaultsGenerator{}
 
 	return &Service{
 		storer: storer,
diff --git a/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator.go b/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator.go
index e1d4d91f..0a27fc74 100644
--- a/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator.go
+++ b/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator.go
@@ -14,7 +14,6 @@ import (
 	"github.com/golang/protobuf/ptypes/duration"
 
 	serverexperimentation "github.com/lyft/clutch/backend/api/chaos/serverexperimentation/v1"
-	ecdsv1 "github.com/lyft/clutch/backend/api/config/module/chaos/serverexperimentation/ecds/v1"
 	"github.com/lyft/clutch/backend/module/chaos/experimentation/xds"
 	"github.com/lyft/clutch/backend/service/chaos/experimentation/experimentstore"
 )
@@ -54,23 +53,11 @@ var DefaultDelayFaultConfig = &gcpFilterCommon.FaultDelay{
 	},
 }
 
-type ECDSServerFaultsResourceGeneratorFactory struct{}
-
-func (ECDSServerFaultsResourceGeneratorFactory) Create(cfg *any.Any) (xds.ECDSResourceGenerator, error) {
-	typedConfig := &ecdsv1.ServerFaultsECDSResourceGeneratorConfig{}
-	err := cfg.UnmarshalTo(typedConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	return &ECDSServerFaultsResourceGenerator{}, nil
-}
-
-type ECDSServerFaultsResourceGenerator struct {
+type ECDSFaultsGenerator struct {
 	SerializedDefaultFaultFilter []byte
 }
 
-func (g *ECDSServerFaultsResourceGenerator) GenerateResource(experiment *experimentstore.Experiment) (*xds.ECDSResource, error) {
+func (g *ECDSFaultsGenerator) GenerateResource(experiment *experimentstore.Experiment) (*xds.ECDSResource, error) {
 	httpFaultConfig, ok := experiment.Config.Message.(*serverexperimentation.HTTPFaultConfig)
 	if !ok {
 		return nil, fmt.Errorf("ECDS server faults generator cannot generate faults for a given experiment (run ID: %s, config ID %s)", experiment.Run.Id, experiment.Config.Id)
@@ -143,7 +130,7 @@ func (g *ECDSServerFaultsResourceGenerator) GenerateResource(experiment *experim
 	return xds.NewECDSResource(enforcingCluster, config)
 }
 
-func (g *ECDSServerFaultsResourceGenerator) GenerateDefaultResource(cluster string, resourceName string) (*xds.ECDSResource, error) {
+func (g *ECDSFaultsGenerator) GenerateDefaultResource(cluster string, resourceName string) (*xds.ECDSResource, error) {
 	if resourceName != faultFilterConfigNameForIngressFault && !strings.HasPrefix(resourceName, fmt.Sprintf(faultFilterConfigNameForEgressFault, "")) {
 		return xds.NewEmptyECDSResource(), nil
 	}
@@ -177,7 +164,7 @@ func (g *ECDSServerFaultsResourceGenerator) GenerateDefaultResource(cluster stri
 	return xds.NewECDSResource(cluster, config)
 }
 
-func (g *ECDSServerFaultsResourceGenerator) createAbortDelayConfig(httpFaultConfig *serverexperimentation.HTTPFaultConfig) (bool, *gcpFilterFault.FaultAbort, *gcpFilterCommon.FaultDelay, error) {
+func (g *ECDSFaultsGenerator) createAbortDelayConfig(httpFaultConfig *serverexperimentation.HTTPFaultConfig) (bool, *gcpFilterFault.FaultAbort, *gcpFilterCommon.FaultDelay, error) {
 	var isEgressFault bool
 	var abort *gcpFilterFault.FaultAbort
 	var delay *gcpFilterCommon.FaultDelay
diff --git a/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator_test.go b/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator_test.go
index dcf0c099..3482754b 100644
--- a/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator_test.go
+++ b/backend/module/chaos/serverexperimentation/xds/ecds_faults_generator_test.go
@@ -17,83 +17,7 @@ import (
 	"github.com/lyft/clutch/backend/service/chaos/experimentation/experimentstore"
 )
 
-const (
-	INTERNAL = "internal"
-	EXTERNAL = "external"
-	ABORT    = "abort"
-	LATENCY  = "latency"
-)
-
-func createExperiment(t *testing.T, upstreamCluster string, downstreamCluster string, faultPercent uint32, faultValue uint32, faultInjectorEnforcing string, faultType string) *experimentstore.Experiment {
-	httpConfig := &serverexperimentation.HTTPFaultConfig{}
-
-	upstreamSingleCluster := &serverexperimentation.SingleCluster{
-		Name: upstreamCluster,
-	}
-
-	downstreamSingleCluster := &serverexperimentation.SingleCluster{
-		Name: downstreamCluster,
-	}
-
-	switch faultType {
-	case ABORT:
-		httpConfig.Fault = &serverexperimentation.HTTPFaultConfig_AbortFault{
-			AbortFault: &serverexperimentation.AbortFault{
-				Percentage:  &serverexperimentation.FaultPercentage{Percentage: faultPercent},
-				AbortStatus: &serverexperimentation.FaultAbortStatus{HttpStatusCode: faultValue},
-			},
-		}
-	case LATENCY:
-		httpConfig.Fault = &serverexperimentation.HTTPFaultConfig_LatencyFault{
-			LatencyFault: &serverexperimentation.LatencyFault{
-				Percentage:      &serverexperimentation.FaultPercentage{Percentage: faultPercent},
-				LatencyDuration: &serverexperimentation.FaultLatencyDuration{FixedDurationMs: faultValue},
-			},
-		}
-	}
-
-	switch faultInjectorEnforcing {
-	case INTERNAL:
-		httpConfig.FaultTargeting = &serverexperimentation.FaultTargeting{
-			Enforcer: &serverexperimentation.FaultTargeting_UpstreamEnforcing{
-				UpstreamEnforcing: &serverexperimentation.UpstreamEnforcing{
-					UpstreamType: &serverexperimentation.UpstreamEnforcing_UpstreamCluster{
-						UpstreamCluster: upstreamSingleCluster,
-					},
-					DownstreamType: &serverexperimentation.UpstreamEnforcing_DownstreamCluster{
-						DownstreamCluster: downstreamSingleCluster,
-					},
-				},
-			},
-		}
-	case EXTERNAL:
-		httpConfig.FaultTargeting = &serverexperimentation.FaultTargeting{
-			Enforcer: &serverexperimentation.FaultTargeting_DownstreamEnforcing{
-				DownstreamEnforcing: &serverexperimentation.DownstreamEnforcing{
-					UpstreamType: &serverexperimentation.DownstreamEnforcing_UpstreamCluster{
-						UpstreamCluster: upstreamSingleCluster,
-					},
-					DownstreamType: &serverexperimentation.DownstreamEnforcing_DownstreamCluster{
-						DownstreamCluster: downstreamSingleCluster,
-					},
-				},
-			},
-		}
-	}
-
-	anyConfig, err := anypb.New(httpConfig)
-	assert.NoError(t, err)
-	jsonConfig, err := protojson.Marshal(anyConfig)
-	assert.NoError(t, err)
-	config, err := experimentstore.NewExperimentConfig("1", string(jsonConfig))
-	assert.NoError(t, err)
-
-	return &experimentstore.Experiment{
-		Config: config,
-	}
-}
-
-func TestServerFaultsECDSResourceGenerating(t *testing.T) {
+func TestECDSFaultsGeneration(t *testing.T) {
 	tests := []struct {
 		experiment                       *experimentstore.Experiment
 		expectedCluster                  string
@@ -179,7 +103,7 @@ func TestServerFaultsECDSResourceGenerating(t *testing.T) {
 		t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
 			t.Parallel()
 
-			g := &ECDSServerFaultsResourceGenerator{}
+			g := &ECDSFaultsGenerator{}
 			r, err := g.GenerateResource(tt.experiment)
 
 			assert.NoError(t, err)
@@ -214,7 +138,7 @@ func TestServerFaultsECDSResourceGenerating(t *testing.T) {
 	}
 }
 
-func TestServerFaultsECDSDefaultResourceGenerating(t *testing.T) {
+func TestECDSDefaultFaultsGeneration(t *testing.T) {
 	tests := []struct {
 		cluster         string
 		resourceName    string
@@ -242,7 +166,7 @@ func TestServerFaultsECDSDefaultResourceGenerating(t *testing.T) {
 		t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
 			t.Parallel()
 
-			g := &ECDSServerFaultsResourceGenerator{}
+			g := &ECDSFaultsGenerator{}
 			r, err := g.GenerateDefaultResource(tt.cluster, tt.resourceName)
 
 			assert.NoError(t, err)
@@ -259,3 +183,79 @@ func TestServerFaultsECDSDefaultResourceGenerating(t *testing.T) {
 		})
 	}
 }
+
+const (
+	INTERNAL = "internal"
+	EXTERNAL = "external"
+	ABORT    = "abort"
+	LATENCY  = "latency"
+)
+
+func createExperiment(t *testing.T, upstreamCluster string, downstreamCluster string, faultPercent uint32, faultValue uint32, faultInjectorEnforcing string, faultType string) *experimentstore.Experiment {
+	httpConfig := &serverexperimentation.HTTPFaultConfig{}
+
+	upstreamSingleCluster := &serverexperimentation.SingleCluster{
+		Name: upstreamCluster,
+	}
+
+	downstreamSingleCluster := &serverexperimentation.SingleCluster{
+		Name: downstreamCluster,
+	}
+
+	switch faultType {
+	case ABORT:
+		httpConfig.Fault = &serverexperimentation.HTTPFaultConfig_AbortFault{
+			AbortFault: &serverexperimentation.AbortFault{
+				Percentage:  &serverexperimentation.FaultPercentage{Percentage: faultPercent},
+				AbortStatus: &serverexperimentation.FaultAbortStatus{HttpStatusCode: faultValue},
+			},
+		}
+	case LATENCY:
+		httpConfig.Fault = &serverexperimentation.HTTPFaultConfig_LatencyFault{
+			LatencyFault: &serverexperimentation.LatencyFault{
+				Percentage:      &serverexperimentation.FaultPercentage{Percentage: faultPercent},
+				LatencyDuration: &serverexperimentation.FaultLatencyDuration{FixedDurationMs: faultValue},
+			},
+		}
+	}
+
+	switch faultInjectorEnforcing {
+	case INTERNAL:
+		httpConfig.FaultTargeting = &serverexperimentation.FaultTargeting{
+			Enforcer: &serverexperimentation.FaultTargeting_UpstreamEnforcing{
+				UpstreamEnforcing: &serverexperimentation.UpstreamEnforcing{
+					UpstreamType: &serverexperimentation.UpstreamEnforcing_UpstreamCluster{
+						UpstreamCluster: upstreamSingleCluster,
+					},
+					DownstreamType: &serverexperimentation.UpstreamEnforcing_DownstreamCluster{
+						DownstreamCluster: downstreamSingleCluster,
+					},
+				},
+			},
+		}
+	case EXTERNAL:
+		httpConfig.FaultTargeting = &serverexperimentation.FaultTargeting{
+			Enforcer: &serverexperimentation.FaultTargeting_DownstreamEnforcing{
+				DownstreamEnforcing: &serverexperimentation.DownstreamEnforcing{
+					UpstreamType: &serverexperimentation.DownstreamEnforcing_UpstreamCluster{
+						UpstreamCluster: upstreamSingleCluster,
+					},
+					DownstreamType: &serverexperimentation.DownstreamEnforcing_DownstreamCluster{
+						DownstreamCluster: downstreamSingleCluster,
+					},
+				},
+			},
+		}
+	}
+
+	anyConfig, err := anypb.New(httpConfig)
+	assert.NoError(t, err)
+	jsonConfig, err := protojson.Marshal(anyConfig)
+	assert.NoError(t, err)
+	config, err := experimentstore.NewExperimentConfig("1", string(jsonConfig))
+	assert.NoError(t, err)
+
+	return &experimentstore.Experiment{
+		Config: config,
+	}
+}
diff --git a/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator.go b/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator.go
index 08ed7177..f3c10cb1 100644
--- a/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator.go
+++ b/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator.go
@@ -3,10 +3,7 @@ package xds
 import (
 	"fmt"
 
-	"github.com/golang/protobuf/ptypes/any"
-
 	serverexperimentationv1 "github.com/lyft/clutch/backend/api/chaos/serverexperimentation/v1"
-	rtdsv1 "github.com/lyft/clutch/backend/api/config/module/chaos/serverexperimentation/rtds/v1"
 	"github.com/lyft/clutch/backend/module/chaos/experimentation/xds"
 	"github.com/lyft/clutch/backend/service/chaos/experimentation/experimentstore"
 )
@@ -33,29 +30,14 @@ const (
 	HTTPStatusForEgress        = `%s.%s.abort.http_status`
 )
 
-type RTDSServerFaultsResourceGenerator struct {
+type RTDSFaultsGenerator struct {
 	// Runtime prefix for ingress faults
 	IngressFaultRuntimePrefix string
 	// Runtime prefix for egress faults
 	EgressFaultRuntimePrefix string
 }
 
-type RTDSServerFaultsResourceGeneratorFactory struct{}
-
-func (RTDSServerFaultsResourceGeneratorFactory) Create(cfg *any.Any) (xds.RTDSResourceGenerator, error) {
-	typedConfig := &rtdsv1.ServerFaultsRTDSResourceGeneratorConfig{}
-	err := cfg.UnmarshalTo(typedConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	return &RTDSServerFaultsResourceGenerator{
-		IngressFaultRuntimePrefix: typedConfig.IngressFaultRuntimePrefix,
-		EgressFaultRuntimePrefix:  typedConfig.EgressFaultRuntimePrefix,
-	}, nil
-}
-
-func (g *RTDSServerFaultsResourceGenerator) GenerateResource(experiment *experimentstore.Experiment) (*xds.RTDSResource, error) {
+func (g *RTDSFaultsGenerator) GenerateResource(experiment *experimentstore.Experiment) (*xds.RTDSResource, error) {
 	httpFaultConfig, ok := experiment.Config.Message.(*serverexperimentationv1.HTTPFaultConfig)
 	if !ok {
 		return nil, fmt.Errorf("RTDS server faults generator cannot generate faults for a given experiment (run ID: %s, config ID %s)", experiment.Run.Id, experiment.Config.Id)
@@ -83,7 +65,7 @@ func (g *RTDSServerFaultsResourceGenerator) GenerateResource(experiment *experim
 	return xds.NewRTDSResource(cluster, keyValuePairs)
 }
 
-func (g *RTDSServerFaultsResourceGenerator) createRuntimeKeys(upstreamCluster string, downstreamCluster string, httpFaultConfig *serverexperimentationv1.HTTPFaultConfig) (string, uint32, string, uint32, error) {
+func (g *RTDSFaultsGenerator) createRuntimeKeys(upstreamCluster string, downstreamCluster string, httpFaultConfig *serverexperimentationv1.HTTPFaultConfig) (string, uint32, string, uint32, error) {
 	var percentageKey string
 	var percentageValue uint32
 	var faultKey string
diff --git a/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator_test.go b/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator_test.go
index 0519fc05..6ab2dbb3 100644
--- a/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator_test.go
+++ b/backend/module/chaos/serverexperimentation/xds/rtds_faults_generator_test.go
@@ -10,7 +10,7 @@ import (
 	"github.com/lyft/clutch/backend/service/chaos/experimentation/experimentstore"
 )
 
-func TestServerFaultsRTDSResourceGenerating(t *testing.T) {
+func TestRTDSFaultsGeneration(t *testing.T) {
 	tests := []struct {
 		experiment               *experimentstore.Experiment
 		ingressRuntimeKeyPrefix  string
@@ -74,7 +74,7 @@ func TestServerFaultsRTDSResourceGenerating(t *testing.T) {
 		t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
 			t.Parallel()
 
-			g := RTDSServerFaultsResourceGenerator{
+			g := RTDSFaultsGenerator{
 				IngressFaultRuntimePrefix: tt.ingressRuntimeKeyPrefix,
 				EgressFaultRuntimePrefix:  tt.egressRuntimeKeyPrefix,
 			}
-- 
GitLab