diff --git a/admiral/pkg/clusters/serviceentry.go b/admiral/pkg/clusters/serviceentry.go index ba583a5b1..ae79ff1e6 100644 --- a/admiral/pkg/clusters/serviceentry.go +++ b/admiral/pkg/clusters/serviceentry.go @@ -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 } @@ -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 } diff --git a/admiral/pkg/clusters/serviceentry_test.go b/admiral/pkg/clusters/serviceentry_test.go index 17d1894a2..9a78e1ea8 100644 --- a/admiral/pkg/clusters/serviceentry_test.go +++ b/admiral/pkg/clusters/serviceentry_test.go @@ -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{})