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

Skip creating ServiceAccount for Rainloop if it is disabled #17

Conversation

bklang
Copy link
Contributor

@bklang bklang commented May 5, 2020

When Rainloop is disabled, we should not create a ServiceAccount for it

@bklang
Copy link
Contributor Author

bklang commented May 5, 2020

A comment on this: I saw the contribution guidelines request tests to be included. I did not attempt that here for a couple of reasons:

  1. (most important) I was unable to install the helm plugin helm-unittest. This may be a problem related to Helm 3, which I am running
  2. While attempting to troubleshoot the above, it appears that the helm-unittest plugin may be abandoned. Several issues are open (including one for the issue I ran into) but no releases have been made
  3. There were no existing tests for this ServiceAccount for me to modify

If you can help with the above, I'm willing to take a shot at tests

@funkypenguin
Copy link
Contributor

Hey @bklang , I appreciate your following up on this.

Yes, the unittest plugin is apparently broken and unmaintained anymore. There's a promising fork I've been following at quintush/helm-unittest#24 - this is likely to become the new repo, IMO.

Even if you manage to get the helm-unittest plugin installed, chances are it'll fail, since some many changes have been introduced in the period since the deprecation of the old helm-unittest plugin.

If you're willing to take a crack it it, I'm happy to support you in this. It's on my "things-I-really-should-tidy-up" backlog ;)

Cheers!
D

@bklang
Copy link
Contributor Author

bklang commented May 6, 2020

Thanks for that pointer. I've gotten the quintush fork of helm-unittest installed and the tests running, but as expected they are throwing a lot of errors. Most of them center around this:

Error: found in requirements.yaml, but missing in charts/ directory: kubernetes-ingress

I've tried helm dep update and helm build, but neither seems to get it going.

@funkypenguin
Copy link
Contributor

I'm working on adding unittests to the GitHub actions CI, so for now let's ignore the testing requirements, since we know there's current breakage. It'll come out in the wash, in time :)

@funkypenguin funkypenguin merged commit a1891b1 into docker-mailserver:master May 6, 2020
@quintush
Copy link

I'm working on adding unittests to the GitHub actions CI, so for now let's ignore the testing requirements, since we know there's current breakage. It'll come out in the wash, in time :)

@funkypenguin,

Thanks for that pointer. I've gotten the quintush fork of helm-unittest installed and the tests running, but as expected they are throwing a lot of errors. Most of them center around this:

Error: found in requirements.yaml, but missing in charts/ directory: kubernetes-ingress

I've tried helm dep update and helm build, but neither seems to get it going.

Hello @bklang, @funkypenguin,
When using helm3, keep in mind for the unittest plugin to use the option -3 (or --helm3).
For backwards compatibility i have added an option to run the plugin with helm2 and helm3.
I can't guarantee no errors will occur, but the error above should be fixed.

@funkypenguin
Copy link
Contributor

Thanks @quintush, I'll give that a go!

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.

3 participants