vidispine / hull

The incredible HULL - Helm Uniform Layer Library - is a Helm library chart to improve Helm chart based workflows
https://github.com/vidispine/hull
Apache License 2.0
226 stars 13 forks source link

Feedback? #43

Open gre9ory opened 3 years ago

gre9ory commented 3 years ago

Hi there!

Please feel free to leave some feedback here e.g. in the comments about this project such as:

Thanks, any feedback is highly appreciated and helps us in improving the project.

JuryA commented 2 years ago

Hi! I found your HULL project and played with it over the weekend. πŸ™‚ ItΒ΄s awesome! On Friday I have an idea to create some smart and useful Helm templates utilizing all advanced techniques possible. I spent a lot of time searching over GitHub, Gitlab, and whole the Internet, but didn't find anything - just theory and β€œdo it yourself” guides... Because I'm very busy to star with some development, I give it a try and try google again - and found your HULL! πŸ™ Looks remarkably good until now - still need some more tests... Also have some points for improvements - I try to open PR or Issue later... πŸ˜‰

matthias4217 commented 2 years ago

Hello, I've started playing with Hull, and I will probably add more comments later on, when I am experienced enough. So far, it's been the first Helm Common Library chart that I've seen adding so much new features to Helm. This is great as it allows to do so much more, but also requires some time to adapt.

I am currently using Hull to reimplement a Chart I've done differently. I've noticed there was no route object (from Openshift) in Hull. If I want to use a Route, what should I do :

gre9ory commented 2 years ago

Hi @JuryA and @matthias4217, sincere apologies for replying so quite late to your comments!

After having opened this issue a year ago I did not check it as often anymore and the notification about your comments unfortunately slipped by me (Covid taking some credit for that πŸ‘Ž). But be assured that I am incredibly happy that you like the project and care about making it better!

@JuryA Please let me know your concrete thoughts on what may be improved and I'll happily look into it or accept at a PR. It sounds very intruiguing to have your input included!

@matthias4217
I totally understand that it takes some time to adapt because the library has grown really in what it can do. The documentation task for that is challenging for me having all the thoughts and functionalities channeled into something that someone get into, follow and understand how to use it. Have you checked the tutorial, my hope was it may lower the barrier of entry somewhat? But first let me answer your question, I think all three of them work with different pros and cons:

Having said this, there is a fourth option which is in my opinion the fastest and maybe a sufficient solution for you: you can use the customresource object type in HULL. This allows you to create any CR object (such as the Route) with HULL, The customresource object type requires you to specify kind and apiVersion of your CRD and provide the contents in the usual spec field. The CRD object to have CR instances accepted can also be deployed as usual in the crd folder of your HULL based chart or it may already exist in your cluster and you don't need to create it (giving that Route is an OpenShift based CRD I am confident every OpenShift cluster will already have it). Only downside to this is that there is no HULL-based JSON schema validation or other treatment of your CR instance configuration but using Β΄customresource you can add any arbitrary CRD instance to your configuration.

Using HULL enables usecases pretty much missing from Helm (and in my opinion not sufficiently replaced by other tools such as an kustomize post-script task). We use as the basis for the usual application-wrapping charts, but meanwhile also often as a "swiss-knife" like tool to create additional project-related K8S objects for our many customer project deployments. Since you can add everything at configuration time you can virtually have a blank canvas HULL chart and in your CD workflow deploy the objects. Definitive pro is you can embedd all your configuration into Helm chart config and don't need to work "out-of-band" by scripting additional kubectl commands etc.

Again, finding your comments made my day so thanks for this πŸ‘ If you have any other thoughts, concerns or if I was babbling too much and still need help just let me know!

JuryA commented 2 years ago

Hi! I'm in rush now, but first, thank you for your reply! πŸ˜‰

... I want to quickly reply to your question:

  • if it is worth adding Route as a standalone object to HULL?

My answer is:

It depends... πŸ€” Openshift is smart enough to simply take standard K8s Ingress resource and then (and ONLY then) if IngressClass is NOT defined, automatically generate Route resources from them. But ... there are a number of caveats that concern if you use for example URL Rewrites or any other fancy and not standardized features. If you need such things to use, then you will definitely need to use Route objects directly. The question is if such things as URL Rewrites are used so often. IMHO use of URL Rewrites should be just in very special corner cases when you don't have other options. Needs of URL Rewrites only be defended if: 1) It's used for some special purpose, 2) is it necessary due to be a part of some complex enterprise structure we cannot change or 3) because it's port of some ultra-optimization. Otherwise, your application has probably the wrong design (the most case).

From my point of view if you want to implement Route object into HULL you should also consider adding also Traefik resources (Middleware, etc.) and de facto any other Ingress Controllers and their respective objects. All of them have the same behavior and are automatically handled - again - if you will use any non-standardized stuff, you must also create appropriate objects manually. So I don't see any difference between Openshift Routes and Ingresses with various IngressClasses.

JuryA commented 2 years ago
  • adding further "non-HULL" templates to your chart and adding configuration for it outside of the hull key in values.yaml is of course still working but so far I have found that all problems that arose I was able to solve within HULL (sometimes requiring implementing additional features) and this adds the most benefit to the project I believe

I definitely agree that everything can be rewritten to pure HULL format and level of complexity (means for humans) but I found some special cases which will be good to discuss. One such idea is to be able to take rendered K8s Manifest and instead send it directly into K8s, just apply for example merge with some other preprepared Manifest. I have some proof of concept to test if it's somehow useful or not.

gre9ory commented 2 years ago

From my point of view if you want to implement Route object into HULL you should also consider adding also Traefik resources (Middleware, etc.) and de facto any other Ingress Controllers and their respective objects. All of them have the same behavior and are automatically handled - again - if you will use any non-standardized stuff, you must also create appropriate objects manually. So I don't see any difference between Openshift Routes and Ingresses with various IngressClasses.

That would be my fear, adding one of the CRD bunch in HULL as an object type may lead to adding (maybe countless) others ... I realize the door is opened with the servicemonitor but adding more object types from CRDs will also increase maintenance effort for the whole thing. Each object type has to be handled in various places (initialization, tests, etc.) so I would tend to keep the number as low as required. If there is a native K8S object type missing still which is required for someones configuration of course that must be added.

Unless there is a strong benefit for a particular CRD from:

I still think the customresource object type is a good flexible solution to create any CR you'd like in your chart.

For example, the Route from here:

apiVersion: route.openshift.io/v1
kind: Route
metadata:
  name: hello-openshift
spec:
  host: hello-openshift-hello-openshift.<Ingress_Domain> 
  port:
    targetPort: 8080
  to:
    kind: Service
    name: hello-openshift

looks almost the same when expressed as a HULL customresource:

hull:
  objects:
    customresource:
      hello-openshift:
        apiVersion: route.openshift.io/v1
        kind: Route
        spec:
          host: hello-openshift-hello-openshift.<Ingress_Domain> 
          port:
            targetPort: 8080
          to:
            kind: Service
            name: hello-openshift

Using transformations on the customresource is also possible, in above example you would probably make that service name dynamic, so you could use instead:

            name: _HT^hello-openshift

and get a name like myrelease-mychart-hello-openshift in the rendered result depending on chart and release name and name override settings.

To give you a different (non-complete) real life example, this is a HULL chart where we create a cert-manager Certificate CR along the lines of:

hull:
  objects:
    ...
    customresource:
      cert-manager-opensearch-certificate:
        staticName: true
        apiVersion: cert-manager.io/v1
        kind: Certificate
        spec:
          isCA: false
          duration: 2160h # 90d
          renewBefore: 360h # 15d
          commonName: "{{ lookup('prepared_config', 'kubernetesPublicEndpoint') }}"
          ...

If you really need that schema validation, some thoughts I have:

Either way you might even get input validation of your CR done in HULL but I haven't tested both possibilities and the effort may outweigh the benefits?

Thanks for the explanation of the Route purpose in OpenShift, I think I see what it is used for now!

I found some special cases which will be good to discuss. One such idea is to be able to take rendered K8s Manifest and instead send it directly into K8s, just apply for example merge with some other preprepared Manifest. I have some proof of concept to test if it's somehow useful or not.

Open for discussion anytime ;)

matthias4217 commented 2 years ago

@gre9ory No problem with the delays. ^^ I've currently made a Hull fork with the implementation of a rough Route object. It's far from being perfect, but enough to work on my proof-of-concept chart using Hull. I hadn't thought of using customresource and I'll look into it next (thanks for sharing an example just now !). Anyway, implementing the route as a Hull object at least made me dig into the code further and learn better how Hull works.

The tutorial has indeed been of help, and I've gotten used of using Hull. I quite enjoy how lightweight my charts are now, and how transformations are really helpful.

gre9ory commented 2 years ago

Hi @matthias4217,

No problem with the delays.

Great :)

I've currently made a Hull fork with the implementation of a rough Route object. It's far from being perfect, but enough to work on my proof-of-concept chart using Hull.

If you publicly share it I can give it a look?

I hadn't thought of using customresource and I'll look into it next (thanks for sharing an example just now !).

Think it can help you achieve what you want in a reasonable way. And if you need more of e.g. CRs of OpenShift CRDs you can just add them the same way as customresource objects without any further implementation. The pros and cons I tried to list above.

Anyway, implementing the route as a Hull object at least made me dig into the code further and learn better how Hull works.

Awesome. I know it can do with some more commenting (which is not handled nicely in Go Templating unfortunately so not actually fun). At least some key parts would be better understandable with comments for sure. Within my company our whole deployment strategy is by now centered around using HULL charts so it will for sure not disappear out of a sudden and over time more colleagues will be brought on board. And if there is more (external) interest in keeping the project going and expanding I will certainly try to invest in more code commenting and it isn't even a huge code base to document in my opinion.

The tutorial has indeed been of help, and I've gotten used of using Hull. I quite enjoy how lightweight my charts are now, and how transformations are really helpful.

Will print that out and put it in a frame in my home office ;) Seriously, that was all I wanted to achieve, truly appreciated!

gre9ory commented 1 year ago

@matthias4217 @JuryA Hi there, been a while :)

Just wanted to check in if you are still happy and if there is something you like to see added featurewise?

I am planning on next adding custom groups for object type defaults. So instead of just having _HULL_OBJECT_TYPE_DEFAULT_ for all instances of a particular type you can define a custom group of default settings and then have individual instances inherit from it. Found that often I have more like several "classes" of defaults I need rather than one all instances should inherit from.

Btw I have realized a long-planned hashsumAnnotation feature where you can automatically mark secrets and configmaps for triggering automatic pod rollover on content changes, more details starting from here.

And in case you haven't noticed, there is also a (rather) new _HT/ shortcut which is really powerful I think because it allows you to trigger include library functions directly without needing to wrap them in any way for HULL. So assuming you have in your work context recurring templating blocks for your charts you can now add your own tpl files, add include functions to them and call them simply with _HT/. It is explained further here.

So hope you are ok, thanks and bye!

sczech commented 1 year ago

Hey @gre9ory, really like your project and I plan to use it as a base for our charts. I'm just wondering if it is possible to set the namespace for the objects inside of values.yaml? Ideally I would like to have a config.general.namespaceOverride key.

gre9ory commented 1 year ago

Hi @sczech

really like your project and I plan to use it as a base for our charts.

Cool, thank you, happy to hear that πŸ‘

I'm just wondering if it is possible to set the namespace for the objects inside of values.yaml? Ideally I would like to have a config.general.namespaceOverride key.

I opened an issue #220 where I am happy to discuss this.

Thanks!