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 a Helm chart for the operator deployment #79

Closed
liyinan926 opened this issue Feb 9, 2018 · 18 comments
Closed

Add a Helm chart for the operator deployment #79

liyinan926 opened this issue Feb 9, 2018 · 18 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@liyinan926
Copy link
Collaborator

No description provided.

@liyinan926 liyinan926 added enhancement New feature or request help wanted Extra attention is needed labels Feb 9, 2018
@skonto
Copy link
Contributor

skonto commented May 9, 2018

Should be added here: https://github.com/kubernetes/charts?

@liyinan926
Copy link
Collaborator Author

Yeah, it could potentially be the place. I guess it will start as an incubator chart?

@skonto
Copy link
Contributor

skonto commented May 9, 2018

Ok will have a look, thanx.

@yuchaoran2011
Copy link
Contributor

A PR is now available at Kubernetes charts repo.

@liyinan926
Copy link
Collaborator Author

Thanks, @yuchaoran2011!

@mrow4a
Copy link
Contributor

mrow4a commented Jul 18, 2018

It would be interesting to use sth like this for webhooks https://github.com/kubernetes/charts/blob/37bf2e2867a124219ce844fd9645fd8f73816583/stable/dex/templates/job.yaml in future Helm Chart. Thoughts?

@liyinan926
Copy link
Collaborator Author

@mrow4a Just saw your comment on using a Job to generate the certificate. I like the idea. @yuchaoran2011 it would be awesome if you can add it into your PR. Thanks!

@yuchaoran2011
Copy link
Contributor

@liyinan926 Ah that's a good idea! I'm on vacation now but I'll update the PR when I'm back.

@mrow4a
Copy link
Contributor

mrow4a commented Jul 27, 2018

@liyinan926 I can also try to figure that out - lets maybe merge what is there first, the other PR is already quite fat.

@mrow4a
Copy link
Contributor

mrow4a commented Jul 27, 2018

@yuchaoran2011 perfect!

@mrow4a
Copy link
Contributor

mrow4a commented Jul 27, 2018

@liyinan926 In fact we can add it as another YAML in manifest right?

@liyinan926
Copy link
Collaborator Author

@mrow4a @yuchaoran2011

I can also try to figure that out - lets maybe merge what is there first, the other PR is already quite fat.

Sounds good.

In fact we can add it as another YAML in manifest right?

Yes, I think so.

@yuchaoran2011
Copy link
Contributor

@mrow4a @liyinan926 Now I have a Docker image that bundles a customized version of gencerts.sh. Then I define a k8s Job in YAML that runs the custom gencerts.sh to generate and install the secret. But the problem is that Helm does not enforce a particular order in which the YAML files are applied, so k8s resources that require the webhook secret can be applied before the secret is created, causing a failure in the chart installation process. I tried making the Job a pre-install hook. But that didn't work because the Job relies on the sparkoperator serviceAccount defined in another YAML file (it uses the token corresponding to the serviceAccount to talk to the API server to install the secret). Any suggestions how I should approach this issue?

@mrow4a
Copy link
Contributor

mrow4a commented Aug 7, 2018

@yuchaoran2011 I think it has to somehow, since you cannot e.g. create spark operator without e.g. sparkoperator namespace. Could you share your progress (changed files) so we could review (WIP)?

@yuchaoran2011
Copy link
Contributor

@mrow4a Sure. I'll clean up things and make a PR today for you guys to review.

@yuchaoran2011
Copy link
Contributor

yuchaoran2011 commented Aug 7, 2018

@mrow4a @liyinan926 The PR is now ready. I also made changes to the Helm chart PR. Please review both places when you get time. Thanks!

@yuchaoran2011
Copy link
Contributor

@liyinan926 Now that the PR is merged, would you want me to update the operator doc to point to the chart as the way to install? Do we still want to keep the /manifest directory?

@liyinan926
Copy link
Collaborator Author

Yes, plase open a PR to update the doc. Thanks! Let's keep the manifest/ directory for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants