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

Implemented Azure e2e tests #352

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Implemented Azure e2e tests #352

merged 1 commit into from
Sep 26, 2024

Conversation

kylewuolle
Copy link
Contributor

Implement Azure template e2e tests. Closes #210.

@kylewuolle kylewuolle added the test e2e Runs the entire provider E2E test suite, controller E2E tests are always ran label Sep 18, 2024
test/e2e/e2e_test.go Outdated Show resolved Hide resolved
test/e2e/e2e_test.go Outdated Show resolved Hide resolved
test/e2e/e2e_test.go Outdated Show resolved Hide resolved
test/e2e/e2e_test.go Show resolved Hide resolved
test/e2e/e2e_test.go Outdated Show resolved Hide resolved
test/managedcluster/azure/azure.go Outdated Show resolved Hide resolved
test/managedcluster/azure/azure.go Outdated Show resolved Hide resolved
test/managedcluster/azure/azure.go Outdated Show resolved Hide resolved
test/managedcluster/providervalidator.go Show resolved Hide resolved
test/e2e/e2e_test.go Show resolved Hide resolved
@squizzi
Copy link
Contributor

squizzi commented Sep 18, 2024

Looking at the latest test failure, my suspicion is the names aren't unique for the cluster names for each provider test and they're running over each other. It might be worth appending a provider name to the end of the ManagedCluster that gets created for each test so they don't clobber each other and race.

@kylewuolle kylewuolle force-pushed the azure-e2e-tests branch 2 times, most recently from 84d25d0 to a8474a7 Compare September 19, 2024 17:31
@kylewuolle kylewuolle force-pushed the azure-e2e-tests branch 7 times, most recently from e3853f2 to e88e451 Compare September 20, 2024 15:08
test/e2e/e2e_test.go Outdated Show resolved Hide resolved
test/managedcluster/azure/azure.go Outdated Show resolved Hide resolved
test/managedcluster/managedcluster.go Outdated Show resolved Hide resolved
@squizzi
Copy link
Contributor

squizzi commented Sep 23, 2024

@kylewuolle In the future can use try not to use force-pushes? At least not after someone has initially reviewed your code. It makes reviewing your code changes after a big initial review harder, you can always just squash the ending result into a single commit.

Copy link
Contributor

@squizzi squizzi left a comment

Choose a reason for hiding this comment

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

lgtm, but a couple of my prior comments are still unresolved, either unchanged or no comment was added to them.

Makefile Outdated Show resolved Hide resolved
}

spec, found, err := unstructured.NestedMap(list.Items[0].Object, "spec")
if !found || err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

The !found || err != nil logic here should be changed, perhaps this was missed in the last round of changes.

Not sure if getAzureInfo can be a GinkgoHelper as well, it doesn't look like it runs in an Eventually.

yamlDoc, err := yamlReader.Read()

if err != nil {
if err == io.EOF {
Copy link
Contributor

Choose a reason for hiding this comment

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

Using errors.Is is the better idea here because errors.Is supports wrapped errors while err == will not.

@Kshatrix
Copy link
Collaborator

@kylewuolle please squash commits like "fix linting error" and let's verify azure part is green before we land it

@squizzi
Copy link
Contributor

squizzi commented Sep 24, 2024

@kylewuolle if you cannot reproduce the latest failure locally -- you can use try to use the tmate action: https://github.com/mxschmitt/action-tmate to live debug what's going on with the infrastructure-aws provider, it'll host a tmate session you can ssh into so you can get into the actions container and figure out potentially what's going on.

My best guess is something is going wacky with the credential creation part, as the CAPA provider won't start if the aws-variables secret can't be mounted, perhaps it's malformed or something.

We might want to modify the e2e test to run collectLogArtifacts during controller test failures as well. I intend to make this change in #360

Copy link
Contributor

@squizzi squizzi left a comment

Choose a reason for hiding this comment

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

As discussed, go ahead and merge what you currently have @kylewuolle and I'm going to work to get the E2E tests all green in #360

@kylewuolle kylewuolle merged commit b199e14 into main Sep 26, 2024
2 of 3 checks passed
@squizzi squizzi deleted the azure-e2e-tests branch October 2, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test e2e Runs the entire provider E2E test suite, controller E2E tests are always ran
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

E2e verification of Azure templates
3 participants