Initial commit for ingress spec

[miles-w-3]

SUMMARY

Adds ingress specs in helm, but needs work to finalize service integration

ISSUE TYPE

• New or Enhanced Feature

ADDITIONAL INFORMATION

[miles-w-3]

@shanemcd I've created this PR at the request of @nickjmv, referenced here. However, I'm brushing up against the problem of reinventing the wheel, since the CRD for awx-operator handles some of this stuff, and I don't want to muddy the waters by adding on top of functionality. It's interesting to think about generally how the roles of operators and helm charts can be really similar, since they both can be responsible for creating new resources.

Initially, I was thinking that this ingress should be activatible when the ingress_type is none so that it doesn't conflict - docs here. The schema I designed was super broad, allowing the user to set their annotations and specs through the values file. However, it still has to integrate with the existing service that is created, for which there is no "none" type - docs here. My plan is just to narrow the schema of the ingress down so that it conforms with the service that will be created by the operator.

However, I think this PR/topic brings to light an issue that will come up whenever we try to add new features to the schema, since they are already designed a certain way. I'm curious if you or any of the other maintainers have thoughts on how in-depth the helm specs should go, since fundamentally they are deviating from the use of the operator. I may add to this PR more in the coming days, but I'm curious on how to address the larger issue going forward, before this is merged. Thanks for your time.

PS @nickjmv, let me know if you have questions about the current template implementation, it is not complete yet but might provide some insight

[nickjmv]

Just checking in here to see if there is an update?

[shanemcd]

Sorry for the slow response here, been busy lately. @miles-w-3 amazing work, as always.

Can you help me understand the purpose of configuring the ingress via helm? I'm confused since it's a resource created and managed by the operator itself. I'll be honest, I'm not a helm user and we dont use it for our product since OpenShift has OperatorHub built in. So if you could shine some light on why this is useful that'd be very much appreciated.

[miles-w-3]

Hi @shanemcd, I appreciate it. I started development on this ticket in response to a request from @nickjmv, and only realized that I will hit a wall once I had begun implementing the features. I have also thought a lot about this since then and my opinion has become more solidified. To shed some light about what helm offers in general, it allows the user to provide a set of values in a yaml file that will be read in by what are essentially go templates to generate valid kubernetes yaml. This is really useful when you want to bundle up a bunch of resources into an application. In that way, they fulfull a small slice of what operators can do - "installations" of a helm app effectively involves applying the template-generated files and storing some metadata in kubernetes to allow the resources to be batch upgraded or uninstalled later on.

[apologies in advance for all of text]

However, as I mentioned, a lot of these roles are already fulfulfilled by the operator. Analogously, the AWX resource reads in all sorts of configurations from the user and uses them to create all manner of resources: Ingresses, services, secrets, etc. Thats where I feel like the missions of the helm chart and the operator may clash if we allow them to. Nevertheless, a lot of people find helm charts useful because they are widely available and offer a single point of installation for any flavor of kubernetes (as long as the manifests are compatible) As I see it, there are effectively 3 philosophies when it comes to developing this chart, given that the application it deploys is an operator:

• Feature parity: Try to replicate the functionality of the operator through helm templates in the case of ingress requests like these
  • The only benefit to this approach is that those familiar with helm and helm configuration likely expect to set up their app directly through the values file and not the AWX resource.
  • The clear downside is huge overhead in terms of feature implementation/updating, documentation, etc.
  • I don't recommend this approach
• AWX configuration: Have values in the AWX operator which ONLY directly map onto the AWX resource
  • This is what I implemented in the spec field within the values - those configs get dumped right into the AWX manifest
  • This allows people to configure their deployment through the helm values
• Fill in the gaps: A hybrid of the two approaches: Any resource that is able to be created/maintained by the Operator should be mapped straight onto AWX and be left alone, but we can add opinionated patterns for accomplishing other functionality.
  • This is the line we've been implicitly walking until now, but would be worth making explicit.
  • As mentioned, this is what I did with external postgres, where the values you use in the config file will be used to create a secret, and it will automatically drop a reference to the secret in the AWX manifest.
    • This fulfills one of the goals of helm, where you have one "go" button
    • I think this is the approach that should be taken, since it allows for us to add convenience features that don't stomp over stuff the operator already tries to do.
  • These extra configs can remain entirely optional and provide opinionated ways of implementing functionality if the user would like to use them. Another example of this would be implementing persistent storage by creating the persistent volume and pvc in helm.
  • The last caveat is that we'd want to keep up to date with what the operator is doing, if the operator implements functionality that the chart is trying to do, the chart should deprecate it and move towards the operator.

I apologize for how long this got, but I figured it's good to be on the same page. All of this to say, implementing an ingress in helm would not fit into approach #3. What we can do is improve documentation on the helm chart to show that any config from the main readme can be slapped into the spec field. Let me know if there's anything I can clarify or summarize more effectively, and thanks.

[nickjmv]

Has there been any progress on this?

[miles-w-3]

The shortest version of that wall of text is that we don't need to implement this directly in the helm chart, since the helm chart creates the AWX resource which can specify the ingress. Is there a reason you can't do something like this @nickjmv, taken straight from the docs:

AWX:
  enabled: true
  spec:
    ingress_type: ingress
    hostname: awx-demo.example.com
    ingress_annotations: |
    environment: testing

[nickjmv]

I'll try it tomorrow!

[john-westcott-iv]

@miles-w-3 Do you happen to be on the #ansible:awx Matrix channel? If so, can you ping me (john-westcott-iv)? I had a couple of questions for you.

[nickjmv]

This worked!

But... Now I got this in the logs:

❯ k logs -f awx-795cf7c897-fmn2f -c awx-task
[wait-for-migrations] Waiting for database migrations...
[wait-for-migrations] Attempt 1 of 30
[wait-for-migrations] Waiting 0.5 seconds before next attempt
[wait-for-migrations] Attempt 2 of 30
[wait-for-migrations] Waiting 1 seconds before next attempt
[wait-for-migrations] Attempt 3 of 30
[wait-for-migrations] Waiting 2 seconds before next attempt
[wait-for-migrations] Attempt 4 of 30
[wait-for-migrations] Waiting 4 seconds before next attempt
[wait-for-migrations] Attempt 5 of 30
[wait-for-migrations] Waiting 8 seconds before next attempt
[wait-for-migrations] Attempt 6 of 30

Any idea what this might be?

• I tried with an empty database, which I hoped would work and I would get a clean AWX instance.
• I tried with a backup of our current AWX (v17) database, but the same logs.

[miles-w-3]

It looks like it is being set up to migrate a database. What is your full AWX deployment/helm values file?

If you want an empty database managed by the operator, at least to start, that should be possible with minimal configuration

[miles-w-3]

Also @john-westcott-iv, fwiw, given the success in creating an ingress through this method I think that option 3 from my post above is a good approach, in the rare cases that the operator does not handle the creation of a resource the helm chart can step in, but otherwise the helm chart should seek to feed directly into the operator. I believe this issue should be closed out, but im happy to update the docs to provide clarity for future contributors

[nickjmv]

@miles-w-3

# Default values for ops-awx-operator.
# This is a YAML-formatted file.
# Declare variables to be passed into your templates.

awx-operator:
  AWX:
    # Paste values of the chart below (at this indentation).
    enabled: true
    name: awx
    spec:
      admin_user: admin
      ingress_type: ingress
      hostname: awx.mas.mobilevikings.be
      ingress_annotations: |
        cert-manager.io/cluster-issuer: letsencrypt-mas-dns

    # configurations for external postgres instance
    postgres:
      enabled: true
      host: XXX.rds.amazonaws.com
      port: 5678
      dbName: awx_test
      username: awx_test
      # for secret management, pass in the password independently of this file
      # at the command line, use --set AWX.postgres.password
      password: XXX
      sslmode: prefer
      type: unmanaged

[miles-w-3]

My guess is that it's not able to authenticate to your db so it's trying to create its own, your config looks correct to me, sorry I cant be of more help

[nickjmv]

I'm having the issue from here: #848

I verified access to the database and that is correct. So I'm not quite sure what is causing the migrations.

[shanemcd]

Apologies for my absence here, been busy with other things for a while.

Also @john-westcott-iv, fwiw, given the success in creating an ingress through this method I think that option 3 from my post above is a good approach, in the rare cases that the operator does not handle the creation of a resource the helm chart can step in, but otherwise the helm chart should seek to feed directly into the operator. I believe this issue should be closed out, but im happy to update the docs to provide clarity for future contributors

I agree with @miles-w-3, going to close this.