Skip to content

charts: Mark all test resources to be only created while running tests#3037

Merged
akshaymankar merged 5 commits intodevelopfrom
helm-hook-test-resources
Jan 31, 2023
Merged

charts: Mark all test resources to be only created while running tests#3037
akshaymankar merged 5 commits intodevelopfrom
helm-hook-test-resources

Conversation

@akshaymankar
Copy link
Member

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jan 30, 2023
Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

As long as CI is happy, this looks good.

I'm not sure configmaps need this annotation, but it probably also doesn't hurt.

@akshaymankar
Copy link
Member Author

I'm not sure configmaps need this annotation, but it probably also doesn't hurt.

I thought why create these resources at all in the non-testing scenarios.

@akshaymankar
Copy link
Member Author

akshaymankar commented Jan 31, 2023

helm test --logs assumes that every test resource is a pod 🤦
helm/helm#11236

@jschaul
Copy link
Member

jschaul commented Jan 31, 2023

helm test --logs assumes that every test resource is a pod facepalm helm/helm#11236

I'm reworking how helm test works in #3040

then this issue goes away. So review and merge that PR first, then this one might go green?

@jschaul
Copy link
Member

jschaul commented Jan 31, 2023

@akshaymankar your latest commit here with patched helm might be in vain: The command "helm test" runs inside the cailleachImage, not the wire-server image, for some reason. Check the wire-server-pr pipeline - I also got bitten by this. Feel free to change this (though maybe then other dependencies are missing)

@akshaymankar
Copy link
Member Author

Ah thanks! I was very confused to see the test fail in exactly the same way. I will change the pipeline I guess.

@jschaul
Copy link
Member

jschaul commented Jan 31, 2023

Ah thanks! I was very confused to see the test fail in exactly the same way. I will change the pipeline I guess.

Or bump the version of helm in use inside cailleach with the same patch, that would also work I suppose.

@akshaymankar
Copy link
Member Author

akshaymankar commented Jan 31, 2023

I vaguely remember wanting to get rid of cailleach-image as much as possible for running integration tests and since we don't rely on any sops machinery anymore we shouldn't need it. I have changed the pipeline already. Hopefully things go green now 🤞

@akshaymankar akshaymankar merged commit 0a8d0a4 into develop Jan 31, 2023
@akshaymankar akshaymankar deleted the helm-hook-test-resources branch January 31, 2023 14:26
@jschaul jschaul mentioned this pull request Feb 1, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments