Allow multiple ingress hosts to be defined when using ingress

[guillaumelfv]

SUMMARY

Deprecate hostname and ingress_tls_secret spec. Add a new spec ingress_hosts which allow multiple ingress hosts to be defined when using ingress.

ingress_hosts:
  - hostname: awx-demo.example.com
    tls_secret: example-com-tls
  - hostname: awx-sample.example.io

• ingress_hosts default to empty and when defined hostname is mandatory but tls_secret is optional.
• ingress_hosts has priority in case both ingress_hosts and hostname/ingress_tls_secret are defined
• hostname and ingress_tls_secret marked as deprecated

For example the above ingress_hosts definition will be render as follow:

spec:
  rules:
  - host: awx-demo.example.com
    http:
      paths:
      - backend:
          service:
            name: awx-service
            port:
              number: 80
        path: /
        pathType: Prefix
  - host: awx-sample.example.io
    http:
      paths:
      - backend:
          service:
            name: awx-service
            port:
              number: 80
        path: /
        pathType: Prefix
  tls:
  - hosts:
    - awx-demo.example.com
    secretName: example-com-tls

fixes #897 #1318

ISSUE TYPE

• New or Enhanced Feature

ADDITIONAL INFORMATION

• Spec hostname is marked as deprecated but still supported
• Spec ingress_tls_secret is marked as deprecated but still supported

This change is backward compatible and priority are as follow based on the spec definition:

hosname	ingress_hosts	Used
defined	undefined	hostname
undefined	defined	ingress_hosts
defined	defined	ingress_hosts

[fciava]

Hi!
Any news on this? It would be really helpful for our company workload.
Thanks

[guillaumelfv]

@fosterseth i rebased and also push the fix for the molecule CI warnings and error. Can we rerun the CI ?

[guillaumelfv]

@fosterseth i am not sure what the CI failure are about.

The PR check https://github.com/ansible/awx-operator/actions/runs/4925590784/jobs/8831262726?pr=1377 just report This job failed with no log output and when i checked what it does, my PR does have New or Enhanced Feature in the issue type body of the PR.

For the molecule timing out not sure why it timeout, I am checking the logs but for now I can not find anything

[guillaumelfv]

Hi, could I get help to have this merged please ?

[guillaumelfv]

Anything blocking the merge of this PR ? Can we run the CI again ?

[guillaumelfv]

@fosterseth @rooftopcellist could I please get at least a new CI run on this MR ? Or any feedback on why it can not be merged ?

[fosterseth]

when I tried your example, I got the following operator error

ingress_hosts:
  - hostname: awx-demo.example.com
    tls_secret: example-com-tls
  - hostname: awx-sample.example.io

The error was: 'dict object' has no attribute 'tls_secret'.

is tls_secret optional or required?

[guillaumelfv]

@fosterseth it was supposed to be optional but it seems there was a mistake in my jinja. I fixed it just now (2331bbc)
and tls_secret should be optional, tested on my side

[fosterseth]

tested again, looks like it works correctly now

[guillaumelfv]

@fosterseth any update ? I just rebased again

[fosterseth]

checked for backward compatibility, which seems to work fine
awx spec

---
apiVersion: awx.ansible.com/v1beta1
kind: AWX
metadata:
  name: awx
spec:
  service_type: nodeport
  ingress_type: ingress
  ingress_tls_secret: example-com-tls
  hostname: awx-demo.example.com

generated ingress

spec:
  rules:
  - host: awx-demo.example.com
    http:
      paths:
      - backend:
          service:
            name: awx-service
            port:
              number: 80
        path: /
        pathType: Prefix
  tls:
  - hosts:
    - awx-demo.example.com
    secretName: example-com-tls

[rooftopcellist]

Can we really mark hostname as deprecated if there is no other way to configure this for ingress_type: Route? I don't see any changes in the part of ingress.yaml.j2 that pertains to Routes.

[guillaumelfv]

Can we really mark hostname as deprecated if there is no other way to configure this for ingress_type: Route? I don't see any changes in the part of ingress.yaml.j2 that pertains to Routes.

@rooftopcellist i do not see anything in roles/installer/templates/networking/ingress.yaml.j2 for the ingress type route using the hostname variable no ? It seems to only use host: {{ route_host }}.

[rooftopcellist]

@guillaumelfv Hey, thank you for following up on this. You are right, hostname is not referenced in Route, so it should be safe to deprecate. Thanks for pointing that out. Can you rebase one last time? Sorry for the churn.

This is ready to merge, we just need to get CI passing.

[rooftopcellist]

I looks like I was able to rebase via the UI, so we can just wait for CI now.

[guillaumelfv]

could we please run the CI again ? I want to see the latest logs to see if i can debug why the molecule tests failed.
@rooftopcellist care you able to run the molecule tests locally ?

[djyasin]

@guillaumelfv Would you mind rebasing one more time for us? Once the conflicts are fixed we are happy to get this merged!

[guillaumelfv]

@djyasin I rebased just now

[djyasin]

@djyasin I rebased just now

Thank you so much! I am running the CI checks now and as long as those are passing I will merge this in!

[guillaumelfv]

@djyasin It timeout, same as any previous run before. According to the logs it silently fail for this task:

60058Z TASK [Create or update the awx.ansible.com/v1alpha1.AWX] ***********************
...
"msg": "\"AWX\" \"example-awx\": Timed out waiting on resource",�[0m
...

Then timeout. I can see the tasks use the following resource template. I will check further on my free time and run the molecule tests locally. Any help from the team, if possible, would be appreciated.

[guillaumelfv]

@djyasin I just push the fix 72ff073. There was an issue when neither hostname or ingress_hosts were defined, which is the scenario the molecule tests use.

In this scenario the ingress rules would be invalid and the controller would throw:

failed: [localhost] (item=networking/ingress) => {"ansible_loop_var": "item", "changed": false, "item": "networking/ingress", "msg": "Failed to apply object: b'{\"kind\":\"Status\",\"apiVersion\":\"v1\",\"metadata\":{},\"status\":\"Failure\",\"message\":\"Ingress.extensions \\\\\"example-awx-ingress\\\\\" is invalid: spec: Invalid value: []networking.IngressRule(nil): either `defaultBackend` or `rules` must be specified\",\"reason\":\"Invalid\",\"details\":{\"name\":\"example-awx-ingress\",\"group\":\"extensions\",\"kind\":\"Ingress\",\"causes\":[{\"reason\":\"FieldValueInvalid\",\"message\":\"Invalid value: []networking.IngressRule(nil): either `defaultBackend` or `rules` must be specified\",\"field\":\"spec\"}]},\"code\":422}\\n'", "reason": "Unprocessable Entity"}

The jinja template needed to be change from:

{% if hostname and (not ingress_hosts) %}
    - host: {{ hostname }}
      http:
         paths:
          - path: '{{ ingress_path }}'
            pathType: '{{ ingress_path_type }}'
            backend:
              service:
                name: '{{ ansible_operator_meta.name }}-service'
                port:
                  number: 80
...

To:

{% if not ingress_hosts %}
    - http:
        paths:
          - path: '{{ ingress_path }}'
            pathType: '{{ ingress_path_type }}'
            backend:
              service:
                name: '{{ ansible_operator_meta.name }}-service'
                port:
                  number: 80
{% if hostname %}
      host: {{ hostname }}
{% endif %}

I did test the fix locally and the molecule tests passed. Sorry it took me so long to fix this regression, once I fixed the molecule tests to pass on my laptop the issue was easy to spot. Just for future reference I face the following running the molecule tests:

objc[18501]: +[__NSCFConstantString initialize] may have been in progress in another thread when fork() was called. We cannot safely call it or ignore it in the fork() child process. Crashing instead. Set a breakpoint on objc_initializeAfterForkError to debug.

And it was fix following this thread rails/rails#38560 and adding:

export OBJC_DISABLE_INITIALIZE_FORK_SAFETY=YES

to my zshrc.

@djyasin could you please trigger a new CI run ?

[djyasin]

@guillaumelfv thank you so much for making these additional changes! I am happy to run those checks again.