Skip to content

Commit

Permalink
Cleanup AWS service configs (#253)
Browse files Browse the repository at this point in the history 
In AWS, we support only one service config. Cleaning up the code
to cater to that. Also, removed unnecessary service config common struct.

Signed-off-by: Rahul Jain <[email protected]>
  • Loading branch information
@reachjainrahul
reachjainrahul authored Jun 21, 2023
1 parent d2a9ed9 commit af8a097
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 329 deletions.
25 changes: 7 additions & 18 deletions pkg/cloudprovider/cloudapi/aws/aws_ec2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,15 @@ func (ec2Cfg *ec2ServiceConfig) getInstanceResourceFilters() ([][]*ec2.Filter, b
func (ec2Cfg *ec2ServiceConfig) getCachedInstances() []*ec2.Instance {
snapshot := ec2Cfg.resourcesCache.GetSnapshot()
if snapshot == nil {
awsPluginLogger().V(4).Info("Cache snapshot nil", "service", awsComputeServiceNameEC2, "account", ec2Cfg.accountNamespacedName)
awsPluginLogger().V(4).Info("Cache snapshot nil", "account", ec2Cfg.accountNamespacedName)
return []*ec2.Instance{}
}
instances := snapshot.(*ec2ResourcesCacheSnapshot).instances
instancesToReturn := make([]*ec2.Instance, 0, len(instances))
for _, instance := range instances {
instancesToReturn = append(instancesToReturn, instance)
}
awsPluginLogger().V(1).Info("Cached vm instances", "service", awsComputeServiceNameEC2, "account", ec2Cfg.accountNamespacedName,
awsPluginLogger().V(1).Info("Cached vm instances", "account", ec2Cfg.accountNamespacedName,
"instances", len(instancesToReturn))
return instancesToReturn
}
Expand All @@ -142,7 +142,7 @@ func (ec2Cfg *ec2ServiceConfig) getManagedVpcIDs() map[string]struct{} {
vpcIDsCopy := make(map[string]struct{})
snapshot := ec2Cfg.resourcesCache.GetSnapshot()
if snapshot == nil {
awsPluginLogger().V(4).Info("Cache snapshot nil", "service", awsComputeServiceNameEC2, "account", ec2Cfg.accountNamespacedName)
awsPluginLogger().V(4).Info("Cache snapshot nil", "account", ec2Cfg.accountNamespacedName)
return vpcIDsCopy
}
vpcIDsSet := snapshot.(*ec2ResourcesCacheSnapshot).vpcIDs
Expand Down Expand Up @@ -191,8 +191,7 @@ func (ec2Cfg *ec2ServiceConfig) getCachedVpcNameToID() map[string]string {
func (ec2Cfg *ec2ServiceConfig) GetCachedVpcs() []*ec2.Vpc {
snapshot := ec2Cfg.resourcesCache.GetSnapshot()
if snapshot == nil {
awsPluginLogger().Info("Cache snapshot nil", "service", awsComputeServiceNameEC2,
"account", ec2Cfg.accountNamespacedName)
awsPluginLogger().Info("Cache snapshot nil", "account", ec2Cfg.accountNamespacedName)
return []*ec2.Vpc{}
}
vpcs := snapshot.(*ec2ResourcesCacheSnapshot).vpcs
Expand Down Expand Up @@ -257,7 +256,7 @@ func (ec2Cfg *ec2ServiceConfig) getInstances() ([]*ec2.Instance, error) {
instances = append(instances, filterInstances...)
}

awsPluginLogger().V(1).Info("Vm instances from cloud", "service", awsComputeServiceNameEC2, "account", ec2Cfg.accountNamespacedName,
awsPluginLogger().V(1).Info("Vm instances from cloud", "account", ec2Cfg.accountNamespacedName,
"instances", len(instances))

return instances, nil
Expand Down Expand Up @@ -318,20 +317,12 @@ func (ec2Cfg *ec2ServiceConfig) GetInternalResourceObjects(namespace string,
vmObjects[vmObject.Name] = vmObject
}

awsPluginLogger().V(1).Info("Internal resource objects", "Service", awsComputeServiceNameEC2, "Account", ec2Cfg.accountNamespacedName,
awsPluginLogger().V(1).Info("Internal resource objects", "account", ec2Cfg.accountNamespacedName,
"VirtualMachine objects", len(vmObjects))

return vmObjects
}

func (ec2Cfg *ec2ServiceConfig) GetName() internal.CloudServiceName {
return awsComputeServiceNameEC2
}

func (ec2Cfg *ec2ServiceConfig) GetType() internal.CloudServiceType {
return internal.CloudServiceTypeCompute
}

func (ec2Cfg *ec2ServiceConfig) GetInventoryStats() *internal.CloudServiceStats {
return ec2Cfg.inventoryStats
}
Expand All @@ -345,7 +336,6 @@ func (ec2Cfg *ec2ServiceConfig) UpdateServiceConfig(newConfig internal.CloudServ
newEc2ServiceConfig := newConfig.(*ec2ServiceConfig)
ec2Cfg.apiClient = newEc2ServiceConfig.apiClient
ec2Cfg.credentials = newEc2ServiceConfig.credentials

return nil
}

Expand Down Expand Up @@ -408,8 +398,7 @@ func (ec2Cfg *ec2ServiceConfig) GetVpcInventory() map[string]*runtimev1alpha1.Vp
vpcMap[strings.ToLower(*vpc.VpcId)] = vpcObj
}

awsPluginLogger().V(1).Info("Cached vpcs", "service", awsComputeServiceNameEC2,
"account", ec2Cfg.accountNamespacedName, "vpc objects", len(vpcMap))
awsPluginLogger().V(1).Info("Cached vpcs", "account", ec2Cfg.accountNamespacedName, "vpc objects", len(vpcMap))

return vpcMap
}
40 changes: 7 additions & 33 deletions pkg/cloudprovider/cloudapi/aws/aws_security.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,13 +680,9 @@ func (c *awsCloud) CreateSecurityGroup(securityGroupIdentifier *securitygroup.Cl
if !found {
return nil, fmt.Errorf("aws account not found managing virtual private cloud [%v]", vpcID)
}
serviceCfg, err := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
if err != nil {
return nil, err
}
ec2Service := serviceCfg.(*ec2ServiceConfig)

cloudSgName := securityGroupIdentifier.GetCloudName(membershipOnly)
ec2Service := accCfg.GetServiceConfig().(*ec2ServiceConfig)
resp, err := ec2Service.createOrGetSecurityGroups(securityGroupIdentifier.Vpc, map[string]struct{}{cloudSgName: {}})
if err != nil {
return nil, err
Expand All @@ -711,17 +707,12 @@ func (c *awsCloud) UpdateSecurityGroupRules(appliedToGroupIdentifier *securitygr
return fmt.Errorf("aws account not found managing virtual private cloud [%v]", vpcID)
}

serviceCfg, err := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
if err != nil {
return err
}
ec2Service := serviceCfg.(*ec2ServiceConfig)

// build from addressGroups, cloudSgNames from rules
cloudSgNames := buildEc2CloudSgNamesFromRules(&appliedToGroupIdentifier.CloudResourceID, append(addIRule, rmIRule...),
append(addERule, rmERule...))

// make sure all required security groups pre-exist
ec2Service := accCfg.GetServiceConfig().(*ec2ServiceConfig)
vpcIDs := []string{vpcID}
vpcPeerIDs := ec2Service.getVpcPeers(vpcID)
vpcIDs = append(vpcIDs, vpcPeerIDs...)
Expand Down Expand Up @@ -786,15 +777,9 @@ func (c *awsCloud) UpdateSecurityGroupMembers(securityGroupIdentifier *securityg
return fmt.Errorf("aws account not found managing virtual private cloud [%v]", vpcID)
}

serviceCfg, err := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
if err != nil {
return err
}
ec2Service := serviceCfg.(*ec2ServiceConfig)

cloudSgName := securityGroupIdentifier.GetCloudName(membershipOnly)

// get addressGroup cloudSgID
cloudSgName := securityGroupIdentifier.GetCloudName(membershipOnly)
ec2Service := accCfg.GetServiceConfig().(*ec2ServiceConfig)
vpcIDs := []string{vpcID}
cloudSgNames := map[string]struct{}{cloudSgName: {}}
out, err := ec2Service.getCloudSecurityGroupsWithNameFromCloud(vpcIDs, cloudSgNames)
Expand Down Expand Up @@ -826,15 +811,10 @@ func (c *awsCloud) DeleteSecurityGroup(securityGroupIdentifier *securitygroup.Cl
return fmt.Errorf("aws account not found managing virtual private cloud [%v]", vpcID)
}

serviceCfg, err := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
if err != nil {
return err
}
ec2Service := serviceCfg.(*ec2ServiceConfig)

// check if sg exists in cloud and get its cloud sg id to delete
vpcIDs := []string{vpcID}
cloudSgNameToDelete := securityGroupIdentifier.GetCloudName(membershipOnly)
ec2Service := accCfg.GetServiceConfig().(*ec2ServiceConfig)
out, err := ec2Service.getCloudSecurityGroupsWithNameFromCloud(vpcIDs, map[string]struct{}{cloudSgNameToDelete: {}})
if err != nil || len(out) == 0 {
return err
Expand Down Expand Up @@ -892,14 +872,8 @@ func (c *awsCloud) GetEnforcedSecurity() []securitygroup.SynchronizationContent
return
}

serviceCfg, err := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
if err != nil {
awsPluginLogger().Error(err, "Enforced-security-cloud-view GET for account skipped", "account", accCfg.GetNamespacedName())
return
}
ec2Service := serviceCfg.(*ec2ServiceConfig)
err = ec2Service.waitForInventoryInit(inventoryInitWaitDuration)
if err != nil {
ec2Service := accCfg.GetServiceConfig().(*ec2ServiceConfig)
if err := ec2Service.waitForInventoryInit(inventoryInitWaitDuration); err != nil {
awsPluginLogger().Error(err, "Enforced-security-cloud-view GET for account skipped", "account", accCfg.GetNamespacedName())
return
}
Expand Down
11 changes: 2 additions & 9 deletions pkg/cloudprovider/cloudapi/aws/aws_services.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ import (
"antrea.io/nephe/pkg/cloudprovider/cloudapi/internal"
)

const (
awsComputeServiceNameEC2 = internal.CloudServiceName("EC2")
)

// awsServiceClientCreateInterface provides interface to create aws service clients.
type awsServiceClientCreateInterface interface {
compute() (awsEC2Wrapper, error)
Expand Down Expand Up @@ -112,12 +108,10 @@ func (h *awsServicesHelperImpl) newServiceSdkConfigProvider(accConfig *awsAccoun
}

func newAwsServiceConfigs(accountNamespacedName *types.NamespacedName, accCredentials interface{}, awsSpecificHelper interface{}) (
[]internal.CloudServiceInterface, error) {
internal.CloudServiceInterface, error) {
awsServicesHelper := awsSpecificHelper.(awsServicesHelper)
awsAccountCredentials := accCredentials.(*awsAccountConfig)

var serviceConfigs []internal.CloudServiceInterface

awsServiceClientCreator, err := awsServicesHelper.newServiceSdkConfigProvider(awsAccountCredentials)
if err != nil {
return nil, err
Expand All @@ -127,7 +121,6 @@ func newAwsServiceConfigs(accountNamespacedName *types.NamespacedName, accCreden
if err != nil {
return nil, err
}
serviceConfigs = append(serviceConfigs, ec2Service)

return serviceConfigs, nil
return ec2Service, nil
}
27 changes: 9 additions & 18 deletions pkg/cloudprovider/cloudapi/aws/aws_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,7 @@ var _ = Describe("AWS cloud", func() {
Expect(err).Should(BeNil())

accCfg, _ := c.cloudCommon.GetCloudAccountByName(&testAccountNamespacedName)
serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
filters := serviceConfig.(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
filters := accCfg.GetServiceConfig().(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
Expect(filters).To(Equal(expectedFilters))
})
})
Expand Down Expand Up @@ -487,8 +486,7 @@ var _ = Describe("AWS cloud", func() {
Expect(err).Should(BeNil())

accCfg, _ := c.cloudCommon.GetCloudAccountByName(&testAccountNamespacedName)
serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
filters := serviceConfig.(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
filters := accCfg.GetServiceConfig().(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
Expect(filters).To(Equal(expectedFilters))
})
It("Should match expected filter - multiple vpcName only match", func() {
Expand Down Expand Up @@ -518,8 +516,7 @@ var _ = Describe("AWS cloud", func() {
Expect(err).Should(BeNil())

accCfg, _ := c.cloudCommon.GetCloudAccountByName(&testAccountNamespacedName)
serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
filters := serviceConfig.(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
filters := accCfg.GetServiceConfig().(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
Expect(filters).To(Equal(expectedFilters))
})
It("Should match expected filter - multiple vpcID & vmName match", func() {
Expand Down Expand Up @@ -573,8 +570,7 @@ var _ = Describe("AWS cloud", func() {
Expect(err).Should(BeNil())

accCfg, _ := c.cloudCommon.GetCloudAccountByName(&testAccountNamespacedName)
serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
filters := serviceConfig.(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
filters := accCfg.GetServiceConfig().(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
Expect(filters).To(Equal(expectedFilters))
})
It("Should match expected filter - multiple with one all", func() {
Expand All @@ -600,8 +596,7 @@ var _ = Describe("AWS cloud", func() {
Expect(err).Should(BeNil())

accCfg, _ := c.cloudCommon.GetCloudAccountByName(&testAccountNamespacedName)
serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
filters := serviceConfig.(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
filters := accCfg.GetServiceConfig().(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
Expect(filters).To(Equal(expectedFilters))
})
It("Should match expected filter - multiple vm names only match", func() {
Expand Down Expand Up @@ -637,8 +632,7 @@ var _ = Describe("AWS cloud", func() {
Expect(err).Should(BeNil())

accCfg, _ := c.cloudCommon.GetCloudAccountByName(&testAccountNamespacedName)
serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
filters := serviceConfig.(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
filters := accCfg.GetServiceConfig().(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
Expect(filters).To(Equal(expectedFilters))
})
It("Should match expected filter - multiple vm IDs only match", func() {
Expand Down Expand Up @@ -674,8 +668,7 @@ var _ = Describe("AWS cloud", func() {
Expect(err).Should(BeNil())

accCfg, _ := c.cloudCommon.GetCloudAccountByName(&testAccountNamespacedName)
serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
filters := serviceConfig.(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
filters := accCfg.GetServiceConfig().(*ec2ServiceConfig).instanceFilters[testSelectorNamespacedName.String()]
Expect(filters).To(Equal(expectedFilters))
})
})
Expand Down Expand Up @@ -726,8 +719,7 @@ func checkAccountAddSuccessCondition(c *awsCloud, namespacedName types.Namespace
return true, errors.New("failed to find account")
}

serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
instances := serviceConfig.(*ec2ServiceConfig).getCachedInstances()
instances := accCfg.GetServiceConfig().(*ec2ServiceConfig).getCachedInstances()
instanceIds := make([]string, 0, len(instances))
for _, instance := range instances {
instanceIds = append(instanceIds, *instance.InstanceId)
Expand All @@ -752,8 +744,7 @@ func checkVpcPollResult(c *awsCloud, namespacedName types.NamespacedName, ids []
return true, errors.New("failed to find account")
}

serviceConfig, _ := accCfg.GetServiceConfigByName(awsComputeServiceNameEC2)
vpcs := serviceConfig.(*ec2ServiceConfig).GetCachedVpcs()
vpcs := accCfg.GetServiceConfig().(*ec2ServiceConfig).GetCachedVpcs()
vpcIDs := make([]string, 0, len(vpcs))
for _, vpc := range vpcs {
_, _ = GinkgoWriter.Write([]byte(fmt.Sprintf("vpc id %s", *vpc.VpcId)))
Expand Down
17 changes: 4 additions & 13 deletions pkg/cloudprovider/cloudapi/azure/azure_compute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (computeCfg *computeServiceConfig) getCachedVirtualMachines() []*virtualMac
instancesToReturn = append(instancesToReturn, virtualMachine)
}

azurePluginLogger().V(1).Info("Cached vm instances", "service", azureComputeServiceNameCompute, "account", computeCfg.account,
azurePluginLogger().V(1).Info("Cached vm instances", "account", computeCfg.account,
"instances", len(instancesToReturn))
return instancesToReturn
}
Expand Down Expand Up @@ -205,7 +205,7 @@ func (computeCfg *computeServiceConfig) getVirtualMachines() ([]*virtualMachineT
}

azurePluginLogger().V(1).Info("Vm instances from cloud",
"service", azureComputeServiceNameCompute, "account", computeCfg.account, "instances", len(virtualMachines))
"account", computeCfg.account, "instances", len(virtualMachines))

return virtualMachines, nil
}
Expand Down Expand Up @@ -296,20 +296,12 @@ func (computeCfg *computeServiceConfig) GetInternalResourceObjects(namespace str
vmObjects[vmObject.Name] = vmObject
}

azurePluginLogger().V(1).Info("Internal resource objects", "Service", azureComputeServiceNameCompute, "Account", computeCfg.account,
azurePluginLogger().V(1).Info("Internal resource objects", "account", computeCfg.account,
"VirtualMachine objects", len(vmObjects))

return vmObjects
}

func (computeCfg *computeServiceConfig) GetName() internal.CloudServiceName {
return azureComputeServiceNameCompute
}

func (computeCfg *computeServiceConfig) GetType() internal.CloudServiceType {
return internal.CloudServiceTypeCompute
}

func (computeCfg *computeServiceConfig) GetInventoryStats() *internal.CloudServiceStats {
return computeCfg.inventoryStats
}
Expand Down Expand Up @@ -396,8 +388,7 @@ func (computeCfg *computeServiceConfig) GetVpcInventory() map[string]*runtimev1a
vpcMap[strings.ToLower(*vpc.ID)] = vpcObj
}
}
azurePluginLogger().V(1).Info("Cached vpcs", "service", azureComputeServiceNameCompute,
"account", computeCfg.account, "vpc objects", len(vpcMap))
azurePluginLogger().V(1).Info("Cached vpcs", "account", computeCfg.account, "vpc objects", len(vpcMap))

return vpcMap
}
Loading

0 comments on commit af8a097

Please sign in to comment.