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

Feedback? #43

Open
gre9ory opened this issue May 31, 2021 · 11 comments
Open

Feedback? #43

gre9ory opened this issue May 31, 2021 · 11 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@gre9ory
Copy link
Collaborator

gre9ory commented May 31, 2021

Hi there!

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

  • what do you think of the concepts and ideas?
  • do plan on using HULL or are actively using it?
  • anything particular you would like to see added or improved?

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

@gre9ory gre9ory added help wanted Extra attention is needed question Further information is requested labels May 31, 2021
@gre9ory gre9ory changed the title Feedback Feedback? May 31, 2021
@gre9ory gre9ory pinned this issue May 9, 2022
@JuryA
Copy link

JuryA commented Aug 29, 2022

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
Copy link

matthias4217 commented Sep 12, 2022

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 :

  • Fork Hull, add a new Route object to the Hull and use this in my chart ? (and maybe to a PR here)
  • Add a second library, which would depend on Hull, and adding the bits (such as a Route object) I need for my other charts in there ?
  • I figure I could also put the Route directly in my application chart, and use the hull.config.general.data to define the custom variables needed in the route

@gre9ory
Copy link
Collaborator Author

gre9ory commented Oct 3, 2022

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:

  • a new "CRD object type" can be added to HULL with some implementation effort, I only did this for Prometheus ServiceMonitor CRDs because they are incredibly popular and deserved an easy configuration and JSON schema validation. This is however the approach requiring most work which pays off if many people benefit from it, since I am not that experienced with OpenShift I can judge if it is worth to add Route as a standalone object to HULL? And sure you can fork it but I am totally into making it work in one place if possible.
  • as probably most advanced option you may add functionality to HULL based charts in form of additional library charts to add HULL-based specification on top of every HULL based chart. For my company I have done this and tried to make a clear cut seperation between generic Kubernetes functionalities in HULL (that hopefully everyone can benefit from) and additional features more specific to our needs (but also with a lot of genericness). If you like to please checkout our [hull-vidispine-addon](https://github.com/vidispine/hull-vidispine-addon) which is exactly this, adding the functionalites to do API calls to custom APIs before and after installation of an applications via HULL. Imagine you need to make some API calls to externals services before an application can be installed and used and/or making some initialization calls to your application after it has started. Deliberately I don't want to publish this as open source because I need to make it work for our use cases specifically but feel free to use it as wanted. Downside to this approach is that in order to apply the additional library values on top of your base HULL-chart configuration you need to merge it during your chart creation workflow with the HULL-chart values.yaml. We accomplish this using jq in the Helm Chart build workflows and the end result is pretty cool, adding domain specific functions on top of your chart.
  • adding further "non-HULL" templates to your chart and add 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

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
Copy link

JuryA commented Oct 3, 2022

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
Copy link

JuryA commented Oct 3, 2022

  • 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
Copy link
Collaborator Author

gre9ory commented Oct 4, 2022

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:

  • having the JSON schema of the CRD implemented in HULL (as with servicemonitor)
  • having non-basic HULL features applied to the CRD such as use of dicts instead of arrays, ... (the hull.base keys of enabled, staticName, annotations and labels are still available for every HULL object including customresource)

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:

  • try add the JSON schemas for the CRDs your chart sports (in addition to the values.schema.json of HULL) to the parent chart values.yaml.json in a structure which integrates with the HULL JSON schemas customresource object definition.
  • or directly add it to HULLs JSON schema of customresource so that given the kind matches Route for a customresource the particular CRDs schema applies - however I am not 100% certain this can be done with the JSON schema. However, this appears a more lightweight solution than adding more full-blown object types for CRD to the mix in terms of maintainability.

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
Copy link

@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
Copy link
Collaborator Author

gre9ory commented Oct 5, 2022

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
Copy link
Collaborator Author

gre9ory commented Mar 31, 2023

@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
Copy link

sczech commented Apr 25, 2023

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
Copy link
Collaborator Author

gre9ory commented Apr 25, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants