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

test: Add NodeClaim e2e tests #9

Merged
merged 4 commits into from
Nov 10, 2023
Merged

test: Add NodeClaim e2e tests #9

merged 4 commits into from
Nov 10, 2023

Conversation

rakechill
Copy link
Contributor

@rakechill rakechill commented Nov 7, 2023

Description

  • These tests were derived from similar tests created for the AWS Karpenter provider.
  • Two tests relied on checking underlying instance state (via VMClient). These will be circled back to in the future once we add support for calling Azure APIs directly via the test environment.

How was this change tested?

  • make test
  • make az-e2etests

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Copy link
Contributor Author

@rakechill rakechill left a comment

Choose a reason for hiding this comment

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

/test

@rakechill
Copy link
Contributor Author

rakechill commented Nov 7, 2023

I'm seeing one case fail due to garbage collection. Investigating now...

{"level":"INFO","time":"2023-11-07T22:36:50.001Z","logger":"controller.nodeclaim.lifecycle","message":"launched nodeclaim","commit":"58975a5-dirty","nodeclaim":"soarerboom-19-lrvave4f6g","nodepool":"","provider-id":"azure:///subscriptions//resourceGroups/mc_karpenter-e2e-nodeclaim-rg-46813590_nodeclaim-mc-46813590_westus2/providers/Microsoft.Compute/virtualMachines/aks-soarerboom-19-lrvave4f6g","instance-type":"Standard_D2pls_v5","zone":"","capacity-type":"on-demand","allocatable":{"cpu":"1900m","ephemeral-storage":"128G","memory":"2014Mi","pods":"110"}}
{"level":"DEBUG","time":"2023-11-07T22:37:03.399Z","logger":"controller.nodeclaim.garbagecollection","message":"garbage collecting nodeclaim with no cloudprovider representation","commit":"58975a5-dirty","nodeclaim":"soarerboom-19-lrvave4f6g","provider-id":"azure:///subscriptions/
/resourceGroups/mc_karpenter-e2e-nodeclaim-rg-46813590_nodeclaim-mc-46813590_westus2/providers/Microsoft.Compute/virtualMachines/aks-soarerboom-19-lrvave4f6g","nodepool":""}

This is coming from karpenter-core's garbage collection controller rather than the azure provider's controller.

Copy link
Contributor Author

@rakechill rakechill left a comment

Choose a reason for hiding this comment

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

/test

Copy link
Collaborator

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

Overall, looking good to me, but have a few questions

test/suites/nodeclaim/suite_test.go Show resolved Hide resolved
test/suites/nodeclaim/suite_test.go Show resolved Hide resolved
test/suites/nodeclaim/nodeclaim_test.go Show resolved Hide resolved
Copy link
Collaborator

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

this should not trigger e2e

Copy link
Collaborator

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

/test

Copy link
Collaborator

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

test

Copy link
Collaborator

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

/test

Copy link
Collaborator

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

/test

@charliedmcb charliedmcb requested review from charliedmcb and removed request for charliedmcb November 9, 2023 22:30
@charliedmcb charliedmcb added go Pull requests that update Go code and removed go Pull requests that update Go code labels Nov 9, 2023
Copy link
Collaborator

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

LGTM.

Wish we could test this within the actual workflow, but your local testing should be good enough. Will mull over creating a better way of testing new E2E suites, since it can't currently be done within the workflow.

@rakechill rakechill merged commit 68986f4 into main Nov 10, 2023
6 checks passed
@rakechill rakechill deleted the rakechill/nodeclaim-e2e branch November 10, 2023 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants