hierarchical-namespaces icon indicating copy to clipboard operation
hierarchical-namespaces copied to clipboard

Create Helm chart as part of the build process

Open rptaylor opened this issue 3 years ago • 40 comments

Hello,

Do you plan to provide a Helm chart for HNC installation? This would make it easier for operators to manage HNC installations.

Thanks!

rptaylor avatar Jun 03 '21 22:06 rptaylor

I'm certainly open to it but I don't really use Helm myself. HNC doesn't really have any options - how would a Helm chart be better than a simple manifest?

Would you be willing to make a PR to add one? If so, I think the way to do it would be similar to how we create the Krew manifest:

  • Create the chart file in the hack/ directory (like the krew version)
  • Add a target to the makefile to generate the chart (like this)
  • Add a stage to the Cloud Build process to upload the chart (like this)

adrianludwin avatar Jun 04 '21 17:06 adrianludwin

A helm chart is useful because it bundles all the components together and allows operations (list, upgrade, uninstall etc.) to be performed on them in aggregate. Currently with one big YAML file, you can install all the components at once with kubectl apply, but then you are left with a slew of objects and resources in your cluster that can not be cohesively managed. How would you uninstall HNC? Currently I think there would be no clean way to do this. You could delete the HNC namespace, but you'd still be left with all the un-namespaced objects like CRDs and ClusterRoles.

Also kubectl apply can not apply negative configuration, so if there are some major changes in a new HNC version say from 0.9 to 1.0, and there are old resource definitions in hnc-manager-0.9.yaml that are not used anymore in hnc-manager-1.0.yaml , they will nevertheless remain in your cluster when you kubectl apply -f hnc-manager-1.0.yaml instead of being removed, adding clutter over time. Once you include something in the HNC YAML file, you will never be able to remove it from the cluster deployments (unless you tell cluster operators to manually kubectl delete things when they upgrade HNC). With Helm, the HNC deployment would be managed so the environment would always be clean, with no leftover resources from previous installations of HNC versions.

Even if there are no configurable HNC options ,

  • will that really always be the case?
  • cluster operators may nevertheless wish to customize some aspects of deployment that are not HNC-specific, but pertain to scaling, production use , or security practices, e.g. set a priorityClass for important cluster apps, increase number of replicas or resource reqs/limits, maybe select their own locally deployed registry mirror to pull images from, apply a PodSecurityPolicy, etc.

Helm provides the flexibility to do that.

I have made a basic Helm chart and might be able to work the HNC YAML into a Helm chart, but I am not sure if it is that simple in this case. It looks like there is a "build" mechanism that uses a makefile to process the YAML with kustomize to produce the output hnc-manager.yaml ? https://github.com/kubernetes-sigs/hierarchical-namespaces/blob/master/Makefile#L138

Normally with Helm I think you would just maintain the YAML templates and chart directly in git , and publish them as they are to a chart repo server, without needing to process or "build" them. There would be no need to combine all the k8s resources into a single YAML file. What is the role of kustomize in the build process?

rptaylor avatar Jun 04 '21 21:06 rptaylor

I'm not sure if it would make sense to add more processing in the Makefile to "unbuild" all the YAML into a Helm chart, when ideally another benefit of Helm would be to simplify the packaging and maintenance of YAML files for the developers and avoid having to do YAML processing. But I don't really know how the build process works, or if you think it would be practical to revamp it.

rptaylor avatar Jun 04 '21 21:06 rptaylor

Noting https://github.com/kubernetes-sigs/multi-tenancy/blob/master/incubator/hnc/docs/user-guide/how-to.md#excluding-namespaces-from-hnc operators might very well want the configurability to exclude additional namespaces: https://github.com/kubernetes-sigs/multi-tenancy/blob/master/incubator/hnc/docs/user-guide/how-to.md#modify-command-line-arguments This may remain true after HNC graduates to GA status.

rptaylor avatar Jun 05 '21 00:06 rptaylor

It seems that HNC does have configurable options that operators may wish to modify, such as the HNCConfiguration object. https://github.com/kubernetes-sigs/multi-tenancy/blob/master/incubator/hnc/docs/user-guide/how-to.md#uninstall-hnc-from-a-cluster Wouldn't any modifications to HNCConfiguration be lost when upgrading HNC with kubectl apply?

With Helm, user-specified properties could easily be integrated together with the essential baseline properties , as values going into the template for the HNCConfiguration object, then applied cohesively in one step.

rptaylor avatar Jun 05 '21 01:06 rptaylor

Excluded namespaces are a good point, I could see you wanting to configure those in Helm. HNCConfiguration isn't really a good point since the default manifest doesn't come with a default copy of HNCConfiguration so it will never be overwritten. You can delete HNC today with kubectl delete -f <manifest> but I agree this is pretty clunky.

None of this is an argument against Helm, btw, just giving you more context so we can solve the problem well.

Kustomize has several roles, only one of which is to collate all the yaml files into one manifest. Some of the yaml files in /config are autogenerated by controller-tools, but they're not complete - they need additional processing, such as adding timeouts to the webhooks. Kustomize lets us apply these in a sane way. It also adds the "hnc-" prefix to all objects (I don't think controller-tools can generate those).

I had a look at what Gatekeeper was doing (like HNC, Gatekeeper uses controller-tools) and it looks like they have a tool to automatically generate their Helm charts from their kustomize output. At a glance, it appears to be a custom-built tool. Maybe we could adapt that? It wouldn't be the first thing we've stolen borrowed from Gatekeeper.

adrianludwin avatar Jun 05 '21 02:06 adrianludwin

Hmm I see, where does the default HNCConfiguration object come from then, is it created by the controller itself if it does not exist already?

more context so we can solve the problem well

Indeed, appreciated.

Hmm I found other cases of people using kubebuilder (controller-tools) wanting to generate helm charts: https://github.com/kubernetes-sigs/kubebuilder/issues/388 https://github.com/kubernetes-sigs/kubebuilder/issues/387#issuecomment-423435974 but there did not seem to be a solution, aside from suggesting to use kustomize to create Helm charts (sounds possible but maybe clunky, I did not see any kustomize functionality which could facilitate that as far as I could find).

Interestingly, helm can invoke kustomize and kustomize can invoke Helm, but both cases appear to be ways to modify a deployment ("chart inflation"), not the reverse of turning YAML into a chart.

In principle I could make a Helm chart based on the current monolithic YAML file but I don't think it would be very useful or practical to do as a one-off manual effort. It seems there are broader considerations involved relating to the whole build process.

rptaylor avatar Jun 07 '21 22:06 rptaylor

Hmm I see, where does the default HNCConfiguration object come from then, is it created by the controller itself if it does not exist already?

Exactly - we report status there, so if the cluster doesn't already have an object we create a blank one. This is the same way we create a HierarchyConfiguration object in any namespace with a child, even if the user didn't create one themselves. But this will never overwrite user data.

Hmm I found other cases of people using kubebuilder (controller-tools) wanting to generate helm charts: kubernetes-sigs/kubebuilder#388 kubernetes-sigs/kubebuilder#387 (comment) but there did not seem to be a solution, aside from suggesting to use kustomize to create Helm charts (sounds possible but maybe clunky, I did not see any kustomize functionality which could facilitate that as far as I could find).

Yeah, that's what I found too :( Gatekeeper seems to have the only viable solution that I've seen, did you look into that?

In principle I could make a Helm chart based on the current monolithic YAML file but I don't think it would be very useful or practical to do as a one-off manual effort.

Agreed, but maybe the GK tool is worth adapting.

Quick Q on Helm: Gatekeeper on seems to keep one version of its Helm chart at a time. Is this typical for Helm? Or do they usually maintain charts for multiple releases?

adrianludwin avatar Jun 08 '21 14:06 adrianludwin

Agreed, but maybe the GK tool is worth adapting.

Maybe. I had a brief look but I'm not a developer per se and don't know Go, so can't say much about helmify but would be nice if it could be used.

Well you could maintain or produce one version of the chart at a time, but then you would probably put it in a chart repo which can include all the previous charts too, like a yum repo. Setting up a basic chart repo is pretty simple, I just made my own served with GH Pages. But for a project like this one might want to automate it and incorporate it in the CI/CD process.

rptaylor avatar Jun 08 '21 19:06 rptaylor

Ok, I'll mark this as a good issue for new contributors, we've had a few people come through lately with interest in this area (e.g. @RealHarshThakur might be interested).

/good-first-issue

adrianludwin avatar Jun 08 '21 21:06 adrianludwin

@adrianludwin: This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to this:

Ok, I'll mark this as a good issue for new contributors, we've had a few people come through lately with interest in this area (e.g. @RealHarshThakur might be interested).

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Jun 08 '21 21:06 k8s-ci-robot

Also /cc @rjbez17

adrianludwin avatar Jun 08 '21 21:06 adrianludwin

I'm good with this, hopefully we can auto-generate it to reduce the release cycles. We'll need some discussions on how we want to handle versioning of the chart in relation to the images. There are a couple approaches, we just need to pick one.

rjbez17 avatar Jun 09 '21 14:06 rjbez17

We don't have anyone signed up to do this work - @rjbez17 , do you know anyone who might be interested?

adrianludwin avatar Jun 09 '21 14:06 adrianludwin

We have some internal desires for this as well so I can find some cycles for it in the next couple of months after the summer vacation/release rush.

rjbez17 avatar Jun 09 '21 15:06 rjbez17

Cool, thanks!

adrianludwin avatar Jun 09 '21 15:06 adrianludwin

I can work on this. However, I'm new to the project so will need a couple of days to understand different components in the current manifest generation.

vnzongzna avatar Jun 14 '21 20:06 vnzongzna

Thanks @vnzongzna , please go ahead. For the purposes of chart creation, you can probably start at line 144, since the results of the earlier lines (e.g. the call to CONTROLLER_GEN) is all checked into the /config directory. So simply inspecting the contents of that directory will probably give you a good idea of what you have to work with.

Thanks for offering to help!

adrianludwin avatar Jun 14 '21 21:06 adrianludwin

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Sep 12 '21 22:09 k8s-triage-robot

/remove-lifecycle stale

rjbez17 avatar Sep 12 '21 23:09 rjbez17

I can work on this if it's not assigned to anybody @adrianludwin

SydMusavi avatar Sep 22 '21 12:09 SydMusavi

I can work on this if it's not assigned to anybody @adrianludwin

Thanks! Can you coordinate with @hutchic to see if he's still involved? Maybe you can start with his work in #62 and create your own PR based off of that?

adrianludwin avatar Sep 22 '21 13:09 adrianludwin

I created this to use by our company as we are always using helm https://github.com/softonic/hierarchical-namespace-controller

santinoncs avatar Sep 30 '21 10:09 santinoncs

Thanks @santinoncs . The big problem here is keeping the chart up-to-date with changes in the config directory, possibly by copying the Gatekeeper solution - see this comment for details.

adrianludwin avatar Oct 07 '21 14:10 adrianludwin

Hello Everyone, I am very much interested in helm charts, kubernetes and would like to ask is this issue fully solved or Can i work on this issue please ?

chetak123 avatar Oct 12 '21 13:10 chetak123

The big barrier here has been making sure that the chart stays up-to-date. E.g. #62 was an initial attempt at doing this but it will require a lot of manual work with each release so the chart's unlikely to be kept up-to-date.

This comment has a discussion about how we could fix this, and if you wanted to work on that, it would be amazing!

adrianludwin avatar Oct 12 '21 14:10 adrianludwin

sorry all we went a different direction. If I locate enough spare cycles / bandwidth I'll look at this again someday but until then feel free to pickup where I left or restart if my direction wasn't the right way to begin

hutchic avatar Oct 12 '21 19:10 hutchic

No worries, thanks! Can you let us know what the direction was? E.g. are you still using HNC but deploying it differently, or did you pick some other solution?

On Tue, Oct 12, 2021 at 3:44 PM Colin Hutchinson @.***> wrote:

sorry all we went a different direction. If I locate enough spare cycles / bandwidth I'll look at this again someday but until then feel free to pickup where I left or restart if my direction wasn't the right way to begin

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/hierarchical-namespaces/issues/46#issuecomment-941376363, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE43PZE7VPTIUYUFM2OAILDUGSF27ANCNFSM46DHGPIQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

adrianludwin avatar Oct 12 '21 19:10 adrianludwin

in other communities the workflow is to create a seperate community repo to maintain the helm-chart - is this a option here ?

haarchri avatar Nov 04 '21 16:11 haarchri

The main thing I'm worried about is keeping it up-to-date. It'll be a bunch of manual work every release and I'm not too confident that people will commit to doing it.

Would a kpt package (https://kpt.dev/) be a feasible option? Those don't rely on templates AIUI so are much easier for the authors to maintain.

On Thu, Nov 4, 2021 at 12:07 PM Christopher Haar @.***> wrote:

in other communities the workflow is to create a seperate community repo to maintain the helm-chart - is this a option here ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/hierarchical-namespaces/issues/46#issuecomment-961190087, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE43PZDEXDQZ4FA456VC5K3UKKVTZANCNFSM46DHGPIQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

adrianludwin avatar Nov 04 '21 16:11 adrianludwin