Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Update test-runner.yaml #320

Merged
merged 3 commits into from
Jan 2, 2020
Merged

Update test-runner.yaml #320

merged 3 commits into from
Jan 2, 2020

Conversation

tehmoon
Copy link
Contributor

@tehmoon tehmoon commented Dec 23, 2019

Add namespace to the tester and force restart it if failing.

@hashicorp-cla
Copy link

hashicorp-cla commented Dec 23, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Hey @tehmoon,

Thanks for this PR! Adding a namespace makes sense, but I'm curious why you are also changing the restartPolicy? This test is meant to run after all pods are in the ready state and provide quick feedback on the state of the cluster. Changing the restart policy means that helm test could hang up to 5min (Kubernetes default timeout for restarts).

@ishustava ishustava added area/chart-only Related to changes that simply require yaml chart changes, e.g. exposing a new field enhancement New feature or request labels Dec 24, 2019
@tehmoon
Copy link
Contributor Author

tehmoon commented Dec 24, 2019

Hi @ishustava.
Thank you for following up!

I also changed the restartPolicy because using this chart with helm template and applying the result to kubernetes directly, I saw that the tester pod was actually being scheduled right away, before the consul pods, making in fail right away.

If that blocks your pipeline, let me know and I'll remove it.

@lkysow
Copy link
Member

lkysow commented Dec 31, 2019 via email

@tehmoon
Copy link
Contributor Author

tehmoon commented Dec 31, 2019

Hi @lkysow,

Yes that's what I'm doing but if one wants to starts the tests, it does not work if using a namespace.
Alternatively, it works if kustomize sets the namespace on top of helm3 template.

@ishustava
Copy link
Contributor

Hey @tehmoon, if you don't mind removing the restartPolicy change (based on what @lkysow said), I can approve your PR. The namespace change looks good.

@tehmoon
Copy link
Contributor Author

tehmoon commented Jan 2, 2020

@ishustava Done!! Thank you so much for the follow up.

Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

🎉

@ishustava ishustava merged commit c57acec into hashicorp:master Jan 2, 2020
@ishustava
Copy link
Contributor

Thank you for making the change @tehmoon and for the PR!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/chart-only Related to changes that simply require yaml chart changes, e.g. exposing a new field enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants