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

Add carvel test to e2e #4489

Merged
merged 9 commits into from
Apr 7, 2022
Merged

Conversation

absoludity
Copy link
Contributor

@absoludity absoludity commented Mar 24, 2022

Signed-off-by: Michael Nelson minelson@vmware.com

Description of the change

See #4093

Benefits

We can have CI checking that as a minimum a simple carvel install works.

Possible drawbacks

Longer CI? We should parallelize the helm and carvel tests.

Applicable issues

Additional information

Creating as draft until CI passes.
Currently failing on CI with:

Running 1 test using 1 worker

     [chromium] › tests/carvel/04-default-deployment.spec.js:8:1 › Deploys package with default values in main cluster
Logging in Kubeapps via OIDC in host: http://kubeapps-ci.kubeapps/
Logged into Kubeapps!
Creating release "test-04-release-42289"

Too long with no output (exceeded 10m0s): context deadline

even though locally it gets past this point.

Feel free to review. I'll try to get it passing tomorrow.

Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity marked this pull request as ready for review March 24, 2022 06:19
@castelblanque
Copy link
Collaborator

Too long with no output (exceeded 10m0s): context deadline exceeded

This is CircleCI killing the process as it has >10m without any output, as the message suggests.

Deployments timeout is set to >> Deployments timeout: 86 mins, >10 of course. So I would say that it is stuck waiting for the deployment to be ready at:

  await page.waitForSelector("css=.application-status-pie-chart-number >> text=1", {
    timeout: utils.getDeploymentTimeout(),
  });
  await page.waitForSelector("css=.application-status-pie-chart-title >> text=Ready", {
    timeout: utils.getDeploymentTimeout(),
  });

Could it be related to #4447 ? The pie chart not being refreshed.

A different issue would be to investigate why the deployment timeout is calculated as 86 mins 😄

@castelblanque
Copy link
Collaborator

Just for the record, if we ever want to increase the no-output timeout (I don't think it is worth for this particular issue), it can be done in CircleCI with no_output_timeout param.
See example here.

@absoludity
Copy link
Contributor Author

Too long with no output (exceeded 10m0s): context deadline exceeded

This is CircleCI killing the process as it has >10m without any output, as the message suggests.

Yep, I realise, just unsure why it happens like that on CI but locally I see more output.

Deployments timeout is set to >> Deployments timeout: 86 mins, >10 of course. So I would say that it is stuck waiting for the deployment to be ready at:

  await page.waitForSelector("css=.application-status-pie-chart-number >> text=1", {
    timeout: utils.getDeploymentTimeout(),
  });
  await page.waitForSelector("css=.application-status-pie-chart-title >> text=Ready", {
    timeout: utils.getDeploymentTimeout(),
  });

Could it be related to #4447 ? The pie chart not being refreshed.

Again, locally I see the pie chart for this one (why I chose it - the issue with #4447 was that the resource refs only included CRDs, not deployments, in the packages I was trying, so we never watched for the resources, hence no pie chart).

A different issue would be to investigate why the deployment timeout is calculated as 86 mins smile

Heh, fun.

Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity
Copy link
Contributor Author

Again, locally I see the pie chart for this one (why I chose it - the issue with #4447 was that the resource refs only included CRDs, not deployments, in the packages I was trying, so we never watched for the resources, hence no pie chart).

The issue was that the check was looking for the pie chart showing 1 pod ready. The carvel package I'm testing with produces 3 pods. It passed locally because one pod came up (and the page updated) before the others came up (and it updated again, but the test had already succeeded).

Let's see if it passes here now.

@absoludity
Copy link
Contributor Author

sigh - still something different in the CI env than from locally where it passes :/

@absoludity
Copy link
Contributor Author

Woo - increasing the no output timeout enabled a failure with an artifact, showing that the test is entering the release name, but then never moving on to select the service account:

failed-ci-carvel

Still keen to know why this can pass locally but not on CI, but checking service accounts etc.

@absoludity
Copy link
Contributor Author

Hah - so locally I was running with a service account login, which meant that the namespace was defaulting to the kubeapps namespace (since the service account token was from there), where the service account kubeapps-operator was available. When running on CI, it ends up in the default namespace, where that service account is not present.

Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity
Copy link
Contributor Author

Carvel test is now passing, just trying to ensure that we don't re-enable packaging.helm when upgrading to support operators, since it causes issues re-installing postgres with the password.

@absoludity
Copy link
Contributor Author

Woo - and it's ready to land :)

Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

Awesome!! It's great we finally have Carvel e2e support, thanks!

@@ -18,6 +18,7 @@ DOCKER_PASSWORD=${6:-""}
TEST_TIMEOUT_MINUTES=${7:-4}
DEX_IP=${8:-"172.18.0.2"}
ADDITIONAL_CLUSTER_IP=${9:-"172.18.0.3"}
KAPP_CONTROLLER_VERSION="v0.32.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm, perhaps we should pass this var also from the circleci config, like we already do for the OLM_VERSION ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops, yes. Forgot to update it. Done.

@castelblanque
Copy link
Collaborator

Carvel test is now passing

Cool!! Great to see it passes OK in CI.

Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity merged commit 18b963f into vmware-tanzu:main Apr 7, 2022
@absoludity absoludity deleted the 4093-e2e-carvel branch April 7, 2022 23:00
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.

Run our e2e tests when using the Carvel plugin
3 participants