Skip to content

Commit

Permalink
Merge pull request #323 from istio-ecosystem/dynamoDB
Browse files Browse the repository at this point in the history 
Remove implementation of deleteWorkloadData and storeWorkloadData
  • Loading branch information
@kpharasi
kpharasi authored Aug 15, 2024
2 parents 0e8cee3 + 7867c4f commit fd3dc71
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 328 deletions.
69 changes: 0 additions & 69 deletions admiral/pkg/clusters/serviceentry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1602,44 +1602,6 @@ func getDNSPrefixFromServiceEntry(seDR *SeDrTuple) string {
}

func deleteWorkloadData(clusterName, env string, serviceEntry *v1alpha3.ServiceEntry, rr *RemoteRegistry, ctxLogger *logrus.Entry) error {
start := time.Now()

if serviceEntry == nil {
return fmt.Errorf("provided service entry is nil")
}

if reflect.DeepEqual(serviceEntry.Spec, networking.ServiceEntry{}) {
return fmt.Errorf("serviceentry %s has a nil spec", serviceEntry.ObjectMeta.Name)
}

if serviceEntry.Spec.Hosts == nil {
return fmt.Errorf("hosts are not defined in serviceentry: %s", serviceEntry.ObjectMeta.Name)
}

if len(serviceEntry.Spec.Hosts) == 0 {
return fmt.Errorf("0 hosts found in serviceentry: %s", serviceEntry.ObjectMeta.Name)
}

if rr.AdmiralDatabaseClient == nil {
return fmt.Errorf("dynamodb client for workload data table is not initialized")
}

workloadDataToDelete := WorkloadData{
AssetAlias: serviceEntry.Annotations[common.GetWorkloadIdentifier()],
Endpoint: serviceEntry.Spec.Hosts[0],
}

err := rr.AdmiralDatabaseClient.Delete(workloadDataToDelete, ctxLogger)
if err != nil {
return err
}

_, ok := rr.AdmiralCache.DynamoDbEndpointUpdateCache.Load(workloadDataToDelete.Endpoint)
if ok {
rr.AdmiralCache.DynamoDbEndpointUpdateCache.Delete(workloadDataToDelete.Endpoint)
}

util.LogElapsedTimeSince("DeleteEndpointRecord", serviceEntry.Spec.Hosts[0], env, clusterName, start)
return nil
}

Expand Down Expand Up @@ -1721,37 +1683,6 @@ func pushWorkloadDataToDynamodbTable(workloadDataToUpdate WorkloadData, endpoint

func storeWorkloadData(clusterName string, serviceEntry *v1alpha3.ServiceEntry,
globalTrafficPolicy *v1.GlobalTrafficPolicy, additionalEndpoints []string, rr *RemoteRegistry, ctxLogger *logrus.Entry, dr networking.DestinationRule, isSuccess bool) error {

start := time.Now()

if serviceEntry == nil {
return fmt.Errorf("provided service entry is nil")
}

if reflect.DeepEqual(serviceEntry.Spec, networking.ServiceEntry{}) {
return fmt.Errorf("serviceentry %s has a nil spec", serviceEntry.ObjectMeta.Name)
}

if serviceEntry.Spec.Hosts == nil {
return fmt.Errorf("hosts are not defined in serviceentry: %s", serviceEntry.ObjectMeta.Name)
}

if len(serviceEntry.Spec.Hosts) == 0 {
return fmt.Errorf("0 hosts found in serviceentry: %s", serviceEntry.ObjectMeta.Name)
}

if rr.AdmiralDatabaseClient == nil {
return fmt.Errorf("dynamodb client for workload data table is not initialized")
}

//get workload data based on service entry, globaltrafficpolicy and additional endpoints
workloadData := getWorkloadData(ctxLogger, serviceEntry, globalTrafficPolicy, additionalEndpoints, dr, clusterName, isSuccess)

err := pushWorkloadDataToDynamodbTable(workloadData, serviceEntry.Spec.Hosts[0], clusterName, rr, ctxLogger)
util.LogElapsedTimeSinceTask(ctxLogger, "UpdateEndpointRecord", serviceEntry.Spec.Hosts[0], "", clusterName, "", start)
if err != nil {
return err
}
return nil
}

Expand Down
260 changes: 1 addition & 259 deletions admiral/pkg/clusters/serviceentry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6192,267 +6192,9 @@ func (mockDatabaseClientWithError) Get(env string, identity string) (interface{}
return nil, fmt.Errorf("failed to get workloadData items")
}

func TestDeleteEndpointDataFromDynamoDB(t *testing.T) {
common.ResetSync()
common.InitializeConfig(admiralParamsForServiceEntryTests())

var se = &v1alpha3.ServiceEntry{
//nolint
Spec: istioNetworkingV1Alpha3.ServiceEntry{
Hosts: []string{"dev.custom.global"},
Endpoints: []*istioNetworkingV1Alpha3.WorkloadEntry{
{
Address: "override.svc.cluster.local",
Ports: map[string]uint32{"http": 80},
Network: "mesh1",
Locality: "us-west",
Weight: 100,
},
},
},
}

var seWithNilSpec = &v1alpha3.ServiceEntry{
ObjectMeta: metav1.ObjectMeta{
Name: "seWithNilSpec",
},
}

var seWithNilSpecHosts = &v1alpha3.ServiceEntry{
ObjectMeta: metav1.ObjectMeta{
Name: "seWithNilSpecHosts",
},
Spec: istioNetworkingV1Alpha3.ServiceEntry{
Addresses: []string{},
},
}

var seWithEmptySpecHosts = &v1alpha3.ServiceEntry{
ObjectMeta: metav1.ObjectMeta{
Name: "seWithEmptySpecHosts",
},
Spec: istioNetworkingV1Alpha3.ServiceEntry{
Hosts: []string{},
},
}

se.Annotations = map[string]string{resourceCreatedByAnnotationLabel: resourceCreatedByAnnotationValue, common.GetWorkloadIdentifier(): "custom"}
se.Labels = map[string]string{"env": "dev"}

rr1, _ := InitAdmiral(context.Background(), admiralParamsForServiceEntryTests())
rr1.AdmiralDatabaseClient = nil

rr2, _ := InitAdmiral(context.Background(), admiralParamsForServiceEntryTests())
rr2.AdmiralDatabaseClient = &mockDatabaseClientWithError{}

testCases := []struct {
name string
serviceEntry *v1alpha3.ServiceEntry
rr *RemoteRegistry
expectedErr bool
expectedErrMsg string
}{
{
name: "Given nil serviceentry object and nil remote registry " +
"When deleteWorkloadData is called, " +
"Then it should return an error",
serviceEntry: nil,
rr: nil,
expectedErr: true,
expectedErrMsg: "provided service entry is nil",
},
{
name: "Given serviceentry object with nil spec" +
"When deleteWorkloadData is called, " +
"Then it should return an error",
serviceEntry: seWithNilSpec,
rr: nil,
expectedErr: true,
expectedErrMsg: "serviceentry seWithNilSpec has a nil spec",
},
{
name: "Given serviceentry object with nil spec hosts and nil remote registry " +
"When deleteWorkloadData is called, " +
"Then it should return an error",
serviceEntry: seWithNilSpecHosts,
rr: nil,
expectedErr: true,
expectedErrMsg: "hosts are not defined in serviceentry: seWithNilSpecHosts",
},
{
name: "Given nil serviceentry object with empty spec hosts and nil remote registry " +
"When deleteWorkloadData is called, " +
"Then it should return an error",
serviceEntry: seWithEmptySpecHosts,
rr: nil,
expectedErr: true,
expectedErrMsg: "0 hosts found in serviceentry: seWithEmptySpecHosts",
},
{
name: "Given serviceentry object and remote registry with nil admiral database client" +
"When deleteWorkloadData is called, " +
"Then it should return an error",
serviceEntry: se,
rr: rr1,
expectedErr: true,
expectedErrMsg: "dynamodb client for workload data table is not initialized",
},
{
name: "Given serviceentry object and remote registry with admiral database client with errors" +
"When deleteWorkloadData is called, " +
"Then it should return an error",
serviceEntry: se,
rr: rr2,
expectedErr: true,
expectedErrMsg: "failed to delete workloadData",
},
}
for _, c := range testCases {
t.Run(c.name, func(t *testing.T) {
err := deleteWorkloadData("testSourceCluster", "testEnv", c.serviceEntry, c.rr, nil)
if err == nil && c.expectedErr {
assert.Fail(t, "expected error to be returned")
} else if c.expectedErrMsg != "" && c.expectedErrMsg != err.Error() {
assert.Failf(t, "actual and expected error do not match. actual - %s, expected %s", err.Error(), c.expectedErrMsg)
}
})
}
}

func TestUpdateEndpointDataFromDynamoDB(t *testing.T) {
func TestDeployRolloutMigration(t *testing.T) {
common.ResetSync()
common.InitializeConfig(admiralParamsForServiceEntryTests())

var se = &v1alpha3.ServiceEntry{
//nolint
Spec: istioNetworkingV1Alpha3.ServiceEntry{
Hosts: []string{"dev.custom.global"},
Endpoints: []*istioNetworkingV1Alpha3.WorkloadEntry{
{
Address: "override.svc.cluster.local",
Ports: map[string]uint32{"http": 80},
Network: "mesh1",
Locality: "us-west",
Weight: 100,
},
},
},
}

se.Annotations = map[string]string{resourceCreatedByAnnotationLabel: resourceCreatedByAnnotationValue, common.GetWorkloadIdentifier(): "custom"}
se.Labels = map[string]string{"env": "dev"}

var seWithNilSpec = &v1alpha3.ServiceEntry{
ObjectMeta: metav1.ObjectMeta{
Name: "seWithNilSpec",
},
}

var seWithNilSpecHosts = &v1alpha3.ServiceEntry{
ObjectMeta: metav1.ObjectMeta{
Name: "seWithNilSpecHosts",
},
Spec: istioNetworkingV1Alpha3.ServiceEntry{
Addresses: []string{},
},
}

var seWithEmptySpecHosts = &v1alpha3.ServiceEntry{
ObjectMeta: metav1.ObjectMeta{
Name: "seWithEmptySpecHosts",
},
Spec: istioNetworkingV1Alpha3.ServiceEntry{
Hosts: []string{},
},
}

rr1, _ := InitAdmiral(context.Background(), admiralParamsForServiceEntryTests())
rr1.AdmiralDatabaseClient = nil

rr2, _ := InitAdmiral(context.Background(), admiralParamsForServiceEntryTests())
rr2.AdmiralDatabaseClient = &mockDatabaseClientWithError{}

testCases := []struct {
name string
serviceEntry *v1alpha3.ServiceEntry
rr *RemoteRegistry
expectedErr bool
expectedErrMsg string
}{
{
name: "Given nil serviceentry object and nil remote registry " +
"When storeWorkloadData is called, " +
"Then it should return an error",
serviceEntry: nil,
rr: nil,
expectedErr: true,
expectedErrMsg: "provided service entry is nil",
},
{
name: "Given serviceentry object with nil spec" +
"When storeWorkloadData is called, " +
"Then it should return an error",
serviceEntry: seWithNilSpec,
rr: nil,
expectedErr: true,
expectedErrMsg: "serviceentry seWithNilSpec has a nil spec",
},
{
name: "Given serviceentry object with nil spec hosts and nil remote registry " +
"When storeWorkloadData is called, " +
"Then it should return an error",
serviceEntry: seWithNilSpecHosts,
rr: nil,
expectedErr: true,
expectedErrMsg: "hosts are not defined in serviceentry: seWithNilSpecHosts",
},
{
name: "Given nil serviceentry object with empty spec hosts and nil remote registry " +
"When storeWorkloadData is called, " +
"Then it should return an error",
serviceEntry: seWithEmptySpecHosts,
rr: nil,
expectedErr: true,
expectedErrMsg: "0 hosts found in serviceentry: seWithEmptySpecHosts",
},
{
name: "Given serviceentry object and remote registry with nil admiral database client" +
"When storeWorkloadData is called, " +
"Then it should return an error",
serviceEntry: se,
rr: rr1,
expectedErr: true,
expectedErrMsg: "dynamodb client for workload data table is not initialized",
},
{
name: "Given serviceentry object and remote registry with admiral database client with errors" +
"When storeWorkloadData is called, " +
"Then it should return an error",
serviceEntry: se,
rr: rr2,
expectedErr: true,
expectedErrMsg: "failed to update workloadData",
},
}

var ctxLogger = logrus.WithFields(logrus.Fields{
"type": "storeWorkloadData",
})

for _, c := range testCases {
t.Run(c.name, func(t *testing.T) {
err := storeWorkloadData("testSourceCluster", c.serviceEntry, nil, []string{}, c.rr, ctxLogger, istioNetworkingV1Alpha3.DestinationRule{}, true)
if err == nil && c.expectedErr {
assert.Fail(t, "expected error to be returned")
} else if c.expectedErrMsg != "" && c.expectedErrMsg != err.Error() {
assert.Failf(t, "actual and expected error do not match. actual - %s, expected %s", err.Error(), c.expectedErrMsg)
}
})
}
}

func TestDeployRolloutMigration(t *testing.T) {
setupForServiceEntryTests()
var (
env = "test"
stop = make(chan struct{})
Expand Down

0 comments on commit fd3dc71

Please sign in to comment.