adding appProtocol to all services

[szelenka]

Checklist:

• Have you added an explanation of what your changes do and why you'd like them to be included?
• Have you updated or added documentation for the change, as applicable?
• Have you tested your changes on all related environments with successful results, as applicable?
• Have you added automated tests?

Type of Changes:

• New feature
• Bug fix
• Documentation
• Testing enhancement
• Other

What is the current behavior (link to any open issues here)?

When the operator generates a new service, it will add the appProtocol field on that service, to allow it to be used by services which look for appProtocol

What is the new behavior (if this is a feature change)?

• Breaking change (fix or feature that would cause existing functionality to change)

Other Information:

Issue: #3435

[cbandy]

Thanks for this! I've been curious about this API field since pgBackRest registered with the IANA at the beginning of this year.

❓ The Istio docs for protocol selection seem to say that Istio will treat protocols it can't detect as TCP. What happens in Istio when all these Services are explicitly set to tcp?

I have a few more questions inline.

[szelenka]

@cbandy when it's set explicitly to tcp nothing much happens TBH. It'll still default to plain TCP.

In the past this was the default we used to enable some of the features in Istio, and some tooling explicitly looks for appProtocol; such as Kiali.

Now that I know there's a IANA for postgres, it makes sense to use that instead. I'll work on updating them

[szelenka]

I also updated the Endpoints, such that they'd copy over the appProtocol from the Service as well

[szelenka]

@cbandy this is my first time contributing to this repo. Not sure what the next steps are after resolving the conversations, can you help guide me?

[cbandy]

I've kicked off some PR checks. I expect an unrelated failure from kubernetes-k3d (latest) but the rest need to pass before we merge.

[cbandy]

@szelenka Thank you for your patience! I have one nitpick on a doc comment.

GitHub didn't run any checks on your last push (not your fault). Do you mind squashing these commits, which will kick them off again?

[cbandy]

I ran into trouble testing this on OCP 4.8. API calls succeed but the appProtocol field isn't saved in the API. The update event(s) trigger another reconcile which writes again which causes more update event(s) causing more reconciles ad nauseam.

[szelenka]

I don't have access to an OCP cluster to debug on.

@cbandy is it just version 4.8, or does it happen on the later builds as well?

This suggests OCP may have just gotten around to "GA" appProtocol in version 4.8.52?
https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable-4.8/changelog.html

[cbandy]

@cbandy is it just version 4.8, or does it happen on the later builds as well?

I only tried on that one version. Looking at the dates, I probably tested on 4.8.51. The cluster updated to 4.8.52 two days after my last comment.

This suggests OCP may have just gotten around to "GA" appProtocol in version 4.8.52? https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable-4.8/changelog.html

That lists all the changes since 4.7.0, so I think you're seeing the fact that OCP 4.8 is based on K8s 1.21. A more granular list is here: https://docs.openshift.com/container-platform/4.8/release_notes/ocp-4-8-release-notes.html#ocp-4-8-asynchronous-errata-updates

I don't have access to an OCP cluster to debug on.

This is the cluster version today. Maybe you can see something similar on another distro?

Server Version: 4.8.52
Kubernetes Version: v1.21.11+5cc9227

The API field was present/available when I tested. I was able to set it using kubectl edit. Maybe something with SSA?

Now that I think about it... the symptoms could also be from an old controller running. Maybe I had a dirty environment.

[szelenka]

I'm not having luck with getting an OpenShift cluster setup.

I've gone through these steps to create a cluster on my Mac with a M1 chip:
https://console.redhat.com/openshift/create/local

But when attempting to run the PGO, it'll assert with:

exec /usr/local/bin/postgres-operator: exec format error

Rather than debugging OpenShift, would it be better to just use the isOpenshift method in the PGO to prevent adding the appProtocol field in OpenShift clusters?