Skip to content

Commit

Permalink
Update for review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@wongni
wongni committed Jun 24, 2022
1 parent 6602856 commit bb9a1a2
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 14 deletions.
11 changes: 8 additions & 3 deletions pkg/dependencies/factory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Dependencies struct {
DockerClient *executables.Docker
Kubectl *executables.Kubectl
Govc *executables.Govc
Cmks map[string]cloudstack.ProviderCmkClient
Cmks map[string]*executables.Cmk
SnowAwsClient aws.Clients
SnowConfigManager *snow.ConfigManager
Writer filewriter.FileWriter
Expand Down Expand Up @@ -266,12 +266,17 @@ func (f *Factory) WithProvider(clusterConfigFile string, clusterConfig *v1alpha1
return fmt.Errorf("unable to get machine config from file %s: %v", clusterConfigFile, err)
}

cmkClientMap := cloudstack.CmkClientMap{}
for name, cmk := range f.dependencies.Cmks {
cmkClientMap[name] = cmk
}

f.dependencies.Provider = cloudstack.NewProvider(
datacenterConfig,
machineConfigs,
clusterConfig,
f.dependencies.Kubectl,
f.dependencies.Cmks,
cmkClientMap,
f.dependencies.Writer,
time.Now,
skipIpCheck,
Expand Down Expand Up @@ -412,7 +417,7 @@ func (f *Factory) WithCmk() *Factory {
if f.dependencies.Cmks != nil {
return nil
}
f.dependencies.Cmks = map[string]cloudstack.ProviderCmkClient{}
f.dependencies.Cmks = map[string]*executables.Cmk{}

execConfig, err := decoder.ParseCloudStackSecret()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/executables/cmk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func NewCmk(executable Executable, writer filewriter.FileWriter, config decoder.
}
}

func (c *Cmk) GetManagementApiEndpoint(ctx context.Context) string {
func (c *Cmk) GetManagementApiEndpoint() string {
return c.config.ManagementUrl
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/providers/cloudstack/cloudstack_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func givenWildcardCmk(mockCtrl *gomock.Controller) ProviderCmkClient {
cmk.EXPECT().ValidateDomainPresent(gomock.Any(), gomock.Any()).AnyTimes()
cmk.EXPECT().ValidateAccountPresent(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
cmk.EXPECT().ValidateNetworkPresent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
cmk.EXPECT().GetManagementApiEndpoint(gomock.Any()).AnyTimes().Return("http://127.16.0.1:8080/client/api")
cmk.EXPECT().GetManagementApiEndpoint().AnyTimes().Return("http://127.16.0.1:8080/client/api")
return cmk
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/providers/cloudstack/mocks/client.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/providers/cloudstack/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type localAvailabilityZone struct {
}

type ProviderCmkClient interface {
GetManagementApiEndpoint(ctx context.Context) string
GetManagementApiEndpoint() string
ValidateCloudStackConnection(ctx context.Context) error
ValidateServiceOfferingPresent(ctx context.Context, zoneId string, serviceOffering anywherev1.CloudStackResourceIdentifier) error
ValidateDiskOfferingPresent(ctx context.Context, zoneId string, diskOffering anywherev1.CloudStackResourceDiskOffering) error
Expand Down Expand Up @@ -96,7 +96,7 @@ func (v *Validator) ValidateCloudStackDatacenterConfig(ctx context.Context, data
if !ok {
return fmt.Errorf("cannot find CloudStack profile for availability zone %s", az.CredentialsRef)
}
endpoint := cmk.GetManagementApiEndpoint(ctx)
endpoint := cmk.GetManagementApiEndpoint()
if endpoint != az.ManagementApiEndpoint {
return fmt.Errorf("cloudstack secret management url (%s) differs from cluster spec management url (%s)",
endpoint, az.ManagementApiEndpoint)
Expand Down
6 changes: 3 additions & 3 deletions pkg/providers/cloudstack/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ func setupMockForDatacenterConfigValidation(cmk *mocks.MockProviderCmkClient, ct
cmk.EXPECT().ValidateDomainPresent(ctx, datacenterConfig.Spec.Domain).AnyTimes().Return(v1alpha1.CloudStackResourceIdentifier{Id: "5300cdac-74d5-11ec-8696-c81f66d3e965", Name: datacenterConfig.Spec.Domain}, nil)
cmk.EXPECT().ValidateAccountPresent(ctx, gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
cmk.EXPECT().ValidateNetworkPresent(ctx, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
cmk.EXPECT().GetManagementApiEndpoint(ctx).AnyTimes().Return(datacenterConfig.Spec.ManagementApiEndpoint)
cmk.EXPECT().GetManagementApiEndpoint().AnyTimes().Return(datacenterConfig.Spec.ManagementApiEndpoint)
}

func setupMockForAvailabilityZonesValidation(cmk *mocks.MockProviderCmkClient, ctx context.Context, azs []v1alpha1.CloudStackAvailabilityZone) {
Expand All @@ -619,7 +619,7 @@ func setupMockForAvailabilityZonesValidation(cmk *mocks.MockProviderCmkClient, c
cmk.EXPECT().ValidateDomainPresent(ctx, az.Domain).AnyTimes().Return(v1alpha1.CloudStackResourceIdentifier{Id: "5300cdac-74d5-11ec-8696-c81f66d3e962", Name: az.Domain}, nil)
cmk.EXPECT().ValidateAccountPresent(ctx, az.Account, gomock.Any()).AnyTimes().Return(nil)
cmk.EXPECT().ValidateNetworkPresent(ctx, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
cmk.EXPECT().GetManagementApiEndpoint(ctx).AnyTimes().Return(az.ManagementApiEndpoint)
cmk.EXPECT().GetManagementApiEndpoint().AnyTimes().Return(az.ManagementApiEndpoint)
}
}

Expand Down Expand Up @@ -890,7 +890,7 @@ func TestValidateMachineConfigsWithAffinity(t *testing.T) {
cmk.EXPECT().ValidateDomainPresent(gomock.Any(), gomock.Any()).AnyTimes()
cmk.EXPECT().ValidateAccountPresent(ctx, gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
cmk.EXPECT().ValidateNetworkPresent(ctx, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
cmk.EXPECT().GetManagementApiEndpoint(ctx).AnyTimes().Return("http://127.16.0.1:8080/client/api")
cmk.EXPECT().GetManagementApiEndpoint().AnyTimes().Return("http://127.16.0.1:8080/client/api")

cmk.EXPECT().ValidateTemplatePresent(ctx, gomock.Any(), gomock.Any(), datacenterConfig.Spec.Account, testTemplate).AnyTimes()
cmk.EXPECT().ValidateServiceOfferingPresent(ctx, gomock.Any(), testOffering).AnyTimes()
Expand Down

0 comments on commit bb9a1a2

Please sign in to comment.