crossplane-contrib / provider-upjet-aws

Official AWS Provider for Crossplane by Upbound.
https://marketplace.upbound.io/providers/upbound/provider-aws
Apache License 2.0
137 stars 113 forks source link

Helm Chart for easier installation #1247

Open candonov opened 3 months ago

candonov commented 3 months ago

What problem are you facing?

When installing and configuring the provider you have to apply 3 yaml files:

There are few specifics that are not straight forward:

  1. There are not enough examples how to configure deploymentruntimeconfig, api documentation is too brief and hence insufficient
  2. One has to know that the Provider should reference the deploymentruntimeconfig name
  3. You cannot apply all 3 yaml files at once because providerconfig.aws.upbound.io/v1beta1 API is set by the Provider, hence the providerconfig has to to be applied only after the provider status is health
  4. For installing the families, you have parametrize the files and use a tool to loop trough the list of families

Uninstalling (kubectl delete) all the yaml files that were applied leaves the upbound-provider-family-aws Provider and pod behind.

It would be useful to be able to generate configurations for each family separately - provide different number of pods, set different resources and limits. And providing IRSA or podidentity recommended aws policies per family.

How could Official AWS Provider help solve your problem?

Create a helm chart for the provider, with elaborate documentation in a values.yaml file.

candonov commented 3 months ago

cc @negz cc @csantanapr, you have anything to add?

negz commented 3 months ago

Thanks @candonov!

We're thinking about this problem, and it's good to have additional context. I'm not sure we want to solve it by using Helm charts to install providers. Ideally we might solve this at the Crossplane package manager level. I've outlined one potential approach in https://github.com/crossplane/crossplane/issues/5259.

negz commented 3 months ago

There are not enough examples how to configure deploymentruntimeconfig, api documentation is too brief and hence insufficient

It might be worth raising a specific documentation PR at https://github.com/crossplane/docs for this - or better yet, opening a PR to improve the documentation.

jdfalk commented 2 months ago

As we deploy providers using a custom helm chart I'd really rather see something provided by the official team rather than pushing this off on the user to try and figure out. @negz Fixing the documentation doesn't fix the fundamental flaw of the runtimeConfig, I just wanted to add a nodeSelector, so I start off just adding

    spec:
      deploymentTemplate:
        spec:
          template:
            spec:
              nodeSelector:
                kubernetes.io/os: linux
                kubernetes.io/arch: amd64

NOPE. That won't work, it errors out saying I need to have the selector, ok cool, I can do that.

     spec:
      deploymentTemplate:
        spec:
          selector:
            matchLabels:
              pkg.crossplane.io/provider: provider-aws-ec2
          template:
            spec:
              nodeSelector:
                kubernetes.io/os: linux
                kubernetes.io/arch: amd64

NOPE. That won't work either. It errors out saying I need spec.deploymentTemplate.spec.template.spec.containers and at this point I've just given up because I've wasted too much time on this and just will find something else that works better. Why would I want to do all this instead of having it do a merge against the defaults. So obviously this isn't ready for anyone to use or it was just designed to fail. I looked at the design doc and the bugs, arguing against sane defaults is like arguing we should give every baby a bowie knife, just crazy.

I'm sure you've seen other CRDs like the alert manager or prometheus ones which allow you to easily define almost all aspects of the deployment without having to copy the entire deployment and repeat the code multiple times. Anyone who has worked with CRDs in literally every other open source tool is going to expect that field to just be a merge of the options set, and the defaults for the provider.

I genuinely like Crossplane, (I've implemented it at the last 4 places I've worked because I like it so much) but this is driving me crazy and I can't believe the team said "hey here's how everyone else does it so it's easy and intuitive to the users but let's not do that", the limitation of making this useful and intuitive to the users isn't a concern to you?

What's worse is that you did this correctly in the serviceAccountTemplate.

      serviceAccountTemplate:
        metadata:
          annotations:
            well-designed: "true"

I don't have to completely spell out every section of the service account, just add the annotation. So please fix this with sane defaults.

Edit: Here's a simple compromise, you have an option inheritDefaults: true which lets you inherit everything and just change the fields you want. That way people who want to redefine everything have the option to do so, and the rest of us can inherit defaults and have behavior that is consistent with most other CRDs.