Fallback to clusterip to determine the service ipfamily

[fedepaol]

K8s versions prior to 1.20 do not have ClusterIPs set, so we fallback to clusterip (singular) to determine the ip family.

Tested locally with
inv dev-env --node-img=kindest/node:v1.19.11@sha256:07db187ae84b4b7de440a73886f008cf903fcf5764ba8106a9fd5243d6f32729 as the CI lanes are running against 1.21.

[fedepaol]

Fixes #1231

[rata]

LGTM, thanks a lot @fedepaol

I haven't tested this, but what you did seems enough :)

[msherif1234]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

[fedepaol]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

That will come for free as in 1.19 there's no dual stack fields, which is exactly the reason why we need to fallback to ClusterIP

[msherif1234]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

That will come for free as in 1.19 there's no dual stack fields, which is exactly the reason why we need to fallback to ClusterIP

we have annotations that someone mistakenly can use with older k8s right ?

[fedepaol]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

That will come for free as in 1.19 there's no dual stack fields, which is exactly the reason why we need to fallback to ClusterIP

we have annotations that someone mistakenly can use with older k8s right ?

Yes, and that would fail because the ip families would not match

[msherif1234]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

That will come for free as in 1.19 there's no dual stack fields, which is exactly the reason why we need to fallback to ClusterIP

we have annotations that someone mistakenly can use with older k8s right ?

Yes, and that would fail because the ip families would not match

maybe worth adding testcase to make sure this should fail if it not already there

[fedepaol]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

That will come for free as in 1.19 there's no dual stack fields, which is exactly the reason why we need to fallback to ClusterIP

we have annotations that someone mistakenly can use with older k8s right ?

Yes, and that would fail because the ip families would not match

maybe worth adding testcase to make sure this should fail if it not already there

We don't have 1.19 in our CI lanes (and I don't think it's worth adding a lane for a k8s version that is not supported)

[msherif1234]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

That will come for free as in 1.19 there's no dual stack fields, which is exactly the reason why we need to fallback to ClusterIP

we have annotations that someone mistakenly can use with older k8s right ?

Yes, and that would fail because the ip families would not match

maybe worth adding testcase to make sure this should fail if it not already there

We don't have 1.19 in our CI lanes (and I don't think it's worth adding a lane for a k8s version that is not supported)

I was asking about unit-test test case

[fedepaol]

I feel if we are in this mode we should fail any attempt to provision svc with dual stack ?

That will come for free as in 1.19 there's no dual stack fields, which is exactly the reason why we need to fallback to ClusterIP

we have annotations that someone mistakenly can use with older k8s right ?

Yes, and that would fail because the ip families would not match

maybe worth adding testcase to make sure this should fail if it not already there

We don't have 1.19 in our CI lanes (and I don't think it's worth adding a lane for a k8s version that is not supported)

I was asking about unit-test test case

Ah then yes, I can add it, and makes sense, you are right.

[msherif1234]

shouldn't we have this function in ipfamily package ?

[fedepaol]

The reason why I put this here was that the ipfamily package is (was) about ips and families and independent from k8s concepts, so it's a bit borderline and felt more like a local util function.
I don't have a strong opinion though, ipfamily.ForService sounds as much as good. Will move it.

[rata]

heh, I started a similar comment, then realized the ipFamily pkg was agnostic of k8s and decided it was fine too.

I am all for ipfamily.ForService, but if you do it and decide it doesn't fit, I'm ok with how the code currently is too :)

[fedepaol]

I just pushed the change :-)

[rata]

LGTM, thanks!