Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Cloudstack multiple endpoint for preflight checks #2559

Merged
merged 25 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bda21a0
Support Cloudstack multiple endpoint for preflight checks
wongni Jun 27, 2022
c464ce9
Update for review comments
wongni Jun 28, 2022
e8e931f
Merge branch 'main' into support-multiple-acs-for-cmk-2
wongni Jun 28, 2022
3efce41
Fix unit test failures
wongni Jun 28, 2022
31679e2
Remove debug print
wongni Jun 28, 2022
461a804
Remove unnecessary file
wongni Jun 28, 2022
dbdac2b
Remove multierror module
wongni Jun 30, 2022
0a87c51
Update to use single cmk object instead of list or map of multiple cmks
wongni Jun 30, 2022
cbd45c9
Remove ValidateCloudStackConnection when cleaning up VMs
wongni Jun 30, 2022
369b53d
Remove NewProviderCustomNet because it's the same as NewProvider
wongni Jun 30, 2022
260ec6e
Avoid using 'failed to' in errors
wongni Jun 30, 2022
5382943
Remove localAvailabilityZones from Validator
wongni Jun 30, 2022
dca6a9e
Remove multipleZone from ValidateNetworkPresent
wongni Jun 30, 2022
074a62c
Return list of errors CleanUpCloudstackTestResources
wongni Jul 1, 2022
bdf278e
Increase unit test coverage
wongni Jul 1, 2022
40c9ddb
Increase unit test coverage #2
wongni Jul 1, 2022
bdbd178
Change error format
wongni Jul 5, 2022
9813e46
Change function names to be more descriptive
wongni Jul 5, 2022
2ddc742
Add default verifySslValue as a constant
wongni Jul 5, 2022
187c194
Fix a bug where empty Zone IDs are used in validateMachineConfig
wongni Jul 5, 2022
0fe30a1
Merge branch 'aws:main' into support-multiple-acs-for-cmk-2
wongni Jul 5, 2022
2ed1d16
Add error message when validating management api endpoint fails
wongni Jul 5, 2022
f6259dd
Make MarkPass message more readable
wongni Jul 5, 2022
3683c61
Merge branch 'aws:main' into support-multiple-acs-for-cmk-2
wongni Jul 6, 2022
222889c
Fix presubmit job failure
wongni Jul 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions internal/test/cleanup/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,20 @@ func CleanUpCloudstackTestResources(ctx context.Context, clusterName string, dry
if err != nil {
return fmt.Errorf("building cmk executable: %v", err)
}
cmk := executableBuilder.BuildCmkExecutable(tmpWriter, *execConfig)
cmk := executableBuilder.BuildCmkExecutable(tmpWriter, execConfig.Profiles)
defer cmk.Close(ctx)

if err := cmk.ValidateCloudStackConnection(ctx); err != nil {
return fmt.Errorf("validating cloudstack connection with cloudmonkey: %v", err)
failedProfiles := []string{}
errors := []error{}
for _, profile := range execConfig.Profiles {
if err := cmk.CleanupVms(ctx, profile.Name, clusterName, dryRun); err != nil {
failedProfiles = append(failedProfiles, profile.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to keep going back and forth on this, it might be good to include the err message itself as well for debugging purposes

errors = append(errors, err)
}
}

return cmk.CleanupVms(ctx, clusterName, dryRun)
if len(failedProfiles) > 0 {
return fmt.Errorf("cleaning up VMs: profiles=%+v, errors=%+v", failedProfiles, errors)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be doing a profile=error mapping instead if that is the intention here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. The new code will return an error with this kind of message.

cleaning up VMs: map[acs#1:Unknown exception acs#2:Unable to reach]

}
return nil
}
3 changes: 2 additions & 1 deletion pkg/dependencies/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,12 +418,13 @@ func (f *Factory) WithCmk() *Factory {
if f.dependencies.Cmk != nil {
return nil
}

execConfig, err := decoder.ParseCloudStackSecret()
if err != nil {
return fmt.Errorf("building cmk executable: %v", err)
}

f.dependencies.Cmk = f.executableBuilder.BuildCmkExecutable(f.dependencies.Writer, *execConfig)
f.dependencies.Cmk = f.executableBuilder.BuildCmkExecutable(f.dependencies.Writer, execConfig.Profiles)
f.dependencies.closers = append(f.dependencies.closers, f.dependencies.Cmk)

return nil
Expand Down
7 changes: 7 additions & 0 deletions pkg/dependencies/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dependencies_test

import (
"context"
"encoding/base64"
"os"
"testing"

Expand All @@ -13,6 +14,7 @@ import (
"github.com/aws/eks-anywhere/pkg/cluster"
"github.com/aws/eks-anywhere/pkg/config"
"github.com/aws/eks-anywhere/pkg/dependencies"
"github.com/aws/eks-anywhere/pkg/providers/cloudstack/decoder"
"github.com/aws/eks-anywhere/pkg/version"
"github.com/aws/eks-anywhere/release/api/v1alpha1"
)
Expand Down Expand Up @@ -107,6 +109,10 @@ func TestFactoryBuildWithClusterManagerWithoutCliConfig(t *testing.T) {
}

func TestFactoryBuildWithMultipleDependencies(t *testing.T) {
configString := test.ReadFile(t, "testdata/cloudstack_config_multiple_profiles.ini")
encodedConfig := base64.StdEncoding.EncodeToString([]byte(configString))
t.Setenv(decoder.EksacloudStackCloudConfigB64SecretKey, encodedConfig)

tt := newTest(t, vsphere)
deps, err := dependencies.NewFactory().
UseExecutableImage("image:1").
Expand All @@ -125,6 +131,7 @@ func TestFactoryBuildWithMultipleDependencies(t *testing.T) {
WithCAPIManager().
WithManifestReader().
WithUnAuthKubeClient().
WithCmk().
Build(context.Background())

tt.Expect(err).To(BeNil())
Expand Down
11 changes: 11 additions & 0 deletions pkg/dependencies/testdata/cloudstack_config_multiple_profiles.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[Global]
verify-ssl = false
api-key = test-key1
secret-key = test-secret1
api-url = http://127.16.0.1:8080/client/api

[Instance2]
verify-ssl = true
api-key = test-key2
secret-key = test-secret2
api-url = http://127.16.0.2:8080/client/api
4 changes: 2 additions & 2 deletions pkg/executables/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func (b *ExecutableBuilder) BuildGovcExecutable(writer filewriter.FileWriter, op
return NewGovc(b.buildExecutable(govcPath), writer, opts...)
}

func (b *ExecutableBuilder) BuildCmkExecutable(writer filewriter.FileWriter, execConfig decoder.CloudStackExecConfig) *Cmk {
return NewCmk(b.buildExecutable(cmkPath), writer, execConfig)
func (b *ExecutableBuilder) BuildCmkExecutable(writer filewriter.FileWriter, configs []decoder.CloudStackProfileConfig) *Cmk {
return NewCmk(b.buildExecutable(cmkPath), writer, configs)
}

func (b *ExecutableBuilder) BuildAwsCli() *AwsCli {
Expand Down
Loading