Create several services routes based on annotations on the svc endpoint

[stuggi]

Creates the route for the keystoneapi, glance, placement, cinder, neutron, nova, heat, horizon, manila and swift based on the annotation set by the service operator on the their service.

The service operator sets for public endpoint the following ingress_create and ingress_name annotation on the service:

apiVersion: v1
kind: Service
metadata:
  annotations:
    core.openstack.org/ingress_create: "true"
    core.openstack.org/ingress_name: glance

• Based on this information and the label selector the openstack-operator passes into the service override on the component, the openstack-operator creates their routes and upsdates the component CR service override with the route hostname for the endpointURL
• with this the openstack-operator does not have to know the service name schema on the service operators
• The core.openstack.org/ingress_name annotation also allows the customization of the route prefix via the service override for the component. The default applied by the service operators is the service name specified in the service operator, e.g to get a route for cinder api my-cinder-openstack.apps-crc.testing:

      cinderAPI:
        override:
          service:
            public:
              metadata:
                annotations:
                  core.openstack.org/ingress_name: my-cinder

• using the service overrides also allows to use a LoadBalancer for the pubic endpoint (cloud even change between route/LoadBalancer service), e.g.

      glanceAPIExternal:
        override:
          service:
            metadata:
              annotations:
                metallb.universe.tf/address-pool: internalapi
                metallb.universe.tf/allow-shared-ip: internalapi
                metallb.universe.tf/loadBalancerIPs: 172.17.0.90
            spec:
              type: LoadBalancer

• Also allows to customize the route via override, so that timeouts or tls settings can be applied/customized.

Depends-On: openstack-k8s-operators/lib-common#332
Depends-On: openstack-k8s-operators/keystone-operator#307
Depends-On: openstack-k8s-operators/glance-operator#309
Depends-On: openstack-k8s-operators/placement-operator#61
Depends-On: openstack-k8s-operators/cinder-operator#262
Depends-On: openstack-k8s-operators/neutron-operator#204
Depends-On: openstack-k8s-operators/nova-operator#522
Depends-On: openstack-k8s-operators/heat-operator#238
Depends-On: openstack-k8s-operators/horizon-operator#212
Depends-On: openstack-k8s-operators/manila-operator#130
Depends-On: openstack-k8s-operators/swift-operator#49
Depends-On: openstack-k8s-operators/infra-operator#119

Jira: OSP-26690

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 457,1e4dc672f8af5adc171000872310194e8551357a

[gibizer]

I'm wondering if we need that the service CR owns the Route. I don't really see a technical issue with this now just feels like we are reaching over the head of the service operator and forcing it to "adopt" a route. What are the pros of having the owner reference pointing to the service CR instead of simply letting the OpenstackControlPlane CR to own the Route it creates.

[stuggi]

the ctlplane CR still owns it as the controller, as mentioned above we just add the service CR to the ownerref list so that we block early deletion and make sure it stays until the service cr is gone

[gibizer]

I see. Is there a specific need for the route to outlive the OpenStackControlPlane CR? I guess the PlacementAPI only owner is also the OpenStackControlPlane CR? So during OpenStackControlPlane deletion both will be deleted anyhow.

[stuggi]

the case where this is required is for keystone. when the osctlplane CR gets deleted, it might immediately start cleaning the resources owned by the osctlplane. when the route for keystone is gone, the cleanup of services/endpoints might fail. Therefore the routes should be cleaned after the service is gone.

[gibizer]

Ohh. Where do we rely on the public keystone endpoint to clean up resources? That seems like a bug. I would assume that the operators use the internal keystone endpoint to do the cleanup.

[stuggi]

the admin service client from [1] is using the public endpoint. its probably ok to switch that to internal. but in general it might be better to keep the route available for a service and how it registered it until it is really gone?

[1] https://github.com/openstack-k8s-operators/keystone-operator/blob/main/api/v1beta1/keystoneapi.go#L78

[gibizer]

I would switch [1] to internal just to be on the safe side. But obviously that is a separate change.
I have no hard opinion about whether to keep or remove the owner ref.

[stuggi]

I'll propose a separate PR to the keystone operator to change it to internal

[dprince]

I really like how this consolidates Route creation logic. One hypothetical concern I can think of is if we were to split out certain services from the OpenStackControlplane (making it more of a core or enabledByDefault set of controlplane services). Like if we were to make the AdvancedNetworking services like Octavia or Designate top level or something like that to make the OpenStackControlplane smaller. If we did that then perhaps we'd make an AdvancedNetworking umbrella CRD in the openstack-operator and could create routes there similarly. Again all this is hypothetical, sort of a what if.

I'd vote we start landing the parts and pieces here in the dependent PRs

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/nova-operator for 522,1e1716beaa6b79a7f96283a3714aeb521afc12d1

[stuggi]

I really like how this consolidates Route creation logic. One hypothetical concern I can think of is if we were to split out certain services from the OpenStackControlplane (making it more of a core or enabledByDefault set of controlplane services). Like if we were to make the AdvancedNetworking services like Octavia or Designate top level or something like that to make the OpenStackControlplane smaller. If we did that then perhaps we'd make an AdvancedNetworking umbrella CRD in the openstack-operator and could create routes there similarly. Again all this is hypothetical, sort of a what if.

I'd vote we start landing the parts and pieces here in the dependent PRs

yes, controllers handling additional CRDs with split out/additional services would then just do the same.

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/placement-operator for 61,486b243c7540ba26b0f37becd7883b5fb38cc198

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/placement-operator for 61,486b243c7540ba26b0f37becd7883b5fb38cc198

[gibizer]

As we discussed we don't want to spend effort creating an abstraction over the Route object due to the cost of maintaining such abstraction. I feel that we will pay that cost in a different form, maintaining nova (and potentially other service operator) logic in openstack-operator.

[gibizer]

I see. Is there a specific need for the route to outlive the OpenStackControlPlane CR? I guess the PlacementAPI only owner is also the OpenStackControlPlane CR? So during OpenStackControlPlane deletion both will be deleted anyhow.

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/nova-operator for 522,60aff336dfc8ad4d44feaa96b265f7ca64ad6fa9

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/neutron-operator for 204,76f931224d25dc0c88c7a93ad8f3f74beb393d20

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/cinder-operator for 262,4363cf90c64d43ce5a57742ef161d7de39f68193

[softwarefactory-project-zuul]

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/cinder-operator for 262,4363cf90c64d43ce5a57742ef161d7de39f68193

[openshift-ci]

New changes are detected. LGTM label has been removed.

[stuggi]

rebased

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/1b51d98b72c94eeaa7a87a3d600a4ed8

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 30m 25s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 56m 51s
❌ cifmw-crc-podified-edpm-baremetal FAILURE in 1h 18m 30s

[stuggi]

/test openstack-operator-build-deploy-kuttl

[rabi]

recheck

Jobs are fixed

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/34f5467eba9b4e78a0a78727114fb69b

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 33m 09s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 45m 38s
❌ cifmw-crc-podified-edpm-baremetal FAILURE in 1h 21m 07s

[rabi]

recheck

[stuggi]

restoring lgtm from @abays , just rebased

[stuggi]

/test openstack-operator-build-deploy-kuttl

[abays]

@stuggi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
Test name Commit Details Required Rerun command
ci/prow/openstack-operator-build-deploy-kuttl a1e296f link true /test openstack-operator-build-deploy-kuttl

Full PR test history. Your PR dashboard.

Hiccup

Removing debug pod ...
error: unable to upgrade connection: container container-00 not found in pod oko-15-x49j4-master-0-debug_openstack-kuttl-tests

/test openstack-operator-build-deploy-kuttl

[abays]

@stuggi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
Test name Commit Details Required Rerun command
ci/prow/openstack-operator-build-deploy-kuttl a1e296f link true /test openstack-operator-build-deploy-kuttl

Full PR test history. Your PR dashboard.

Same transient error as last time.

/test openstack-operator-build-deploy-kuttl

[rabi]

What's the reason for this change? Should not we set conditions to unknown at the beginning of each reconcile? Won't conditions give confusing picture if reconcile fails earlier than the last failure?