Bump serving, Eventing deps to 1.3.0

[skonto]

Proposed Changes

• Bumps Serving, Eventing to 1.3.
• Deps like Knative Operator can be updated in a subsequent PR as there are some breaking changes in that package.
  For example several types moved to a base pkg etc.
• Split into several commits for easier review.

[skonto]

This can be removed if we update the operator in another PR.

[skonto]

The k8s.io replacements probably can be eliminated when we bump Eventing too.

[pierDipi]

I'd rather bump every component dependencies in one go.

[nak3]

LGTM if CI passed.

But... Actually I still don't know how this PR was created 😅 The README.md says:

To update the dependencies, update the KN_VERSION variable in update-deps.sh. Then run ./hack/update-deps.sh --upgrade (like we do upstream) to pull in all the correct versions. It's encouraged to commit the generated changes separately, to ease review.

But I cannot create the same change with this PR by ./hack/update-deps.sh --upgrade and I cannot find the procedure in https://github.com/openshift-knative/serverless-release-manual as well.

Maybe SIG Release team knows how to create this PR? 😅 It would be great if we can find the docs for "How to Bump deps in S-O".

[pierDipi]

IMHO, dependencies should be bumped for every component together and we can remove replace directives.

[skonto]

I am in favor of this but I think the last time we didnt want to do a big PR for all deps. In the past I did this in one PR. Let me try it anyway.

[pierDipi]

That's how upstream works as well, every component is always aligned

[skonto]

Yes as I mentioned I prefer that, I will bump Eventing too. I added the replacements so Eventing can do the rest of the bumping anyway, that's all.

[skonto]

@nak3 @pierDipi
When updating Eventing and removing replacements I hit multiple times:

# knative.dev/pkg/client/injection/kube/client
../../../vendor/knative.dev/pkg/client/injection/kube/client/client.go:205:5: cannot use (*wrapClient)(nil) (type *wrapClient) as type kubernetes.Interface in assignment:
	*wrapClient does not implement kubernetes.Interface (missing AutoscalingV2 method)

Reason is: this Serving PR knative/serving@4126928 updated K8s version on release-1.3 branch to v0.23.4 while on release-1.3 on knative.dev/pkg we are still on v0.22.5 https://github.com/knative/pkg/blob/a637a96a1bd980f099d76d07e7a965847114700e/go.mod#L46. This causes this failure.
I guess we need to keep replacements in this PR, fix Serving and then remove replacements. WDYTH?

[pierDipi]

@nak3 @pierDipi When updating Eventing and removing replacements I hit multiple times:

# knative.dev/pkg/client/injection/kube/client
../../../vendor/knative.dev/pkg/client/injection/kube/client/client.go:205:5: cannot use (*wrapClient)(nil) (type *wrapClient) as type kubernetes.Interface in assignment:
	*wrapClient does not implement kubernetes.Interface (missing AutoscalingV2 method)

Reason is: this Serving PR knative/serving@4126928 updated K8s version on release-1.3 branch to v0.23.4 while on release-1.3 on knative.dev/pkg we are still on v0.22.5 https://github.com/knative/pkg/blob/a637a96a1bd980f099d76d07e7a965847114700e/go.mod#L46. This causes this failure. I guess we need to keep replacements in this PR, fix Serving and then remove replacements. WDYTH?

I'd fix/bump serving first

[skonto]

I'd fix/bump serving first

Basically there is nothing to fix as Serving on release-1.3 uses the replacements anyway check bellow here: https://github.com/knative/serving/blob/ac292339e627fbc3f2acd3feefbdbd09350cc7e8/go.mod#L47-L50. So I dont think we can avoid them. I misread initially and tried to remove the replacements (thus the error) but I shouldn't.

[pierDipi]

what's the reason for using a previous minor k8s version?
if we cannot bump k8s versions, the eventing bump won't work ...

[skonto]

what's the reason for using a previous k8s version?

I use whatever exists per brunch, each release branch is cut against a specific K8s version afaik. Eventing on release-1.3 should use the same no? It seems it does: https://github.com/knative/eventing/blob/release-1.3/go.mod#L42 v0.22.5.

if we cannot bump k8s versions, the eventing bump won't work ...

Could you explain more? Here the plan for release-1.3 is to use v0.22.5. Is that problematic in some way? Which part uses v0.23.4? Eventing does not seem to use v0.23.4 just Serving (due to that PR and that is why we need the replacements):

github.com/google/go-containerregistry/pkg/authn/k8schain@v0.0.0-20220414154538-570ba6c88a50 k8s.io/api@v0.23.4
github.com/google/go-containerregistry/pkg/authn/k8schain@v0.0.0-20220414154538-570ba6c88a50 k8s.io/client-go@v0.23.4
github.com/google/go-containerregistry/pkg/authn/k8schain@v0.0.0-20220414154538-570ba6c88a50 k8s.io/apimachinery@v0.23.4
github.com/google/go-containerregistry/pkg/authn/kubernetes@v0.0.0-20220414143355-892d7a808387 k8s.io/api@v0.23.4
github.com/google/go-containerregistry/pkg/authn/kubernetes@v0.0.0-20220414143355-892d7a808387 k8s.io/apimachinery@v0.23.4
github.com/google/go-containerregistry/pkg/authn/kubernetes@v0.0.0-20220414143355-892d7a808387 k8s.io/client-go@v0.23.4

[pierDipi]

I see, then with that commit in serving we won't get #1602 (comment) anymore, right?

[skonto]

I see, then with that commit in serving we won't get #1602 (comment) anymore, right?

The replacements exist both upstream and downstream on release-1.3 branch for Serving eg. https://github.com/openshift/knative-serving/blob/release-v1.3/go.mod. But in S-O we have to use them again otherwise 0.23.4 is being picked up.
Using at the S-O:

	k8s.io/api v0.23.4 => k8s.io/api v0.22.5
	k8s.io/apimachinery v0.23.4 => k8s.io/apimachinery v0.22.5
	k8s.io/client-go v0.23.4 => k8s.io/client-go v0.22.5

makes that client-go error above to go away.
I updated the PR let's see.

[pierDipi]

So, in my opinion, this repository (ideally) shouldn't have any replace directive for transitive dependencies and just inherit whatever serving/eventing use that would simplify the deps bump on every release because we don't need to update the replace directive each time (and we did miss this step in a past release, so to avoid it at all we should keep this step simple).

[pierDipi]

It seems wrong to me that to fix serving (or whatever other component) we're hacking this repository, that's why I'm saying let's align serving/eventing to whatever works here, that's cleaner because the fix is localized to a specific component.

[skonto]

It seems wrong to me that to fix serving (or whatever other component) we're hacking this repository, that's why I'm saying let's align serving/eventing to whatever works here, that's cleaner because the fix is localized to a specific component.

Serving upstream had to update stuff as a backport and then added these replacements directives due to that google auth package for good or bad. This makes S-O to pick up the wrong version sicne it has no knowledge of the replacement directives at the Serving side.
I dont expect this to happen for 1.4 check here: https://github.com/knative/serving/blob/release-1.4/go.mod, Serving on release-1.4 is back to normal again, uses v0.23.5. So this is an exception imho. What alternative do you see (we don't want to revert the upstream backport because it fixes the tag resolution which is pretty important)? From my pov there is nothing to fix unless we find a different version for github.com/google/go-containerregistry that uses 0.22.5 and we backport upstream/downstream.
I doubt it because Dave would have already done so but anyway and without the right version probably the upstream registry fix wont work.

However, I dont see why Serving/Eventing are not aligned here? Both use v0.22.5 on release-1.3. These are temporary replacements as I explained that will go away with release-1.4.

[pierDipi]

It seems wrong to me that to fix serving (or whatever other component) we're hacking this repository, that's why I'm saying let's align serving/eventing to whatever works here, that's cleaner because the fix is localized to a specific component.

Serving upstream had to update stuff as a backport and then added these replacements directives due to that google auth package for good or bad. This makes S-O to pick up the wrong version sicne it has no knowledge of the replacement directives at the Serving side. I dont expect this to happen for 1.4 check here: https://github.com/knative/serving/blob/release-1.4/go.mod, Serving on release-1.4 is back to normal again, uses v0.23.5. So this is an exception imho. What alternative do you see (we don't want to revert the upstream backport because it fixes the tag resolution which is pretty important)? From my pov there is nothing to fix unless we find a different version for github.com/google/go-containerregistry that uses 0.22.5 and we backport upstream/downstream. I doubt it because Dave would have already done so but anyway and without the right version probably the upstream registry fix wont work.

However, I dont see why Serving/Eventing are not aligned here? Both use v0.22.5 on release-1.3. These are temporary replacements as I explained that will go away with release-1.4.

Is serverless operator involved in that feature? afaik, that fix is only needed in the serving controller which isn't built from this repository, so I don't get why replace directives here should fix that issue.

[skonto]

so I don't get why replace directives here should fix that issue.

It does not fix that issue but S-O depends on Serving pkgs. Without the replacement directives and when we update deps to release-1.3 (go mod etc) will pick the version 0.23.4 for k8s.io apis, client go etc. Then that means if you try compile S-O you will get:

# knative.dev/pkg/client/injection/kube/client
../../../vendor/knative.dev/pkg/client/injection/kube/client/client.go:205:5: cannot use (*wrapClient)(nil) (type *wrapClient) as type kubernetes.Interface in assignment:
	*wrapClient does not implement kubernetes.Interface (missing AutoscalingV2 method)

We get that error because although Eventing, Serving use 0.22.5 S-O gets an upgrade to 0.23.4. For some reason go does not see the replacement directives in Serving (expected probably) and enforces 0.23.4. In order to fix that one obvious way is to add the directives in S-O too. We are not fixing the registry issue in S-O. We are just fixing the incompatibility of K8s.io versions for S-O itself.
I was trying to explain that the registry issue upstream led to the addition of the replacement directives at the Serving repo.
If you check Serving repos upstream/downstream they introduce 0.23.4 with replacement directives and do not use directly 0.22.5. 0.23.4 is different compared to 0.22.5.

[pierDipi]

Ok, thanks for sharing the details and for the bump!

/lgtm
/hold

/unhold if it's ready to go in.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, skonto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [skonto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[skonto]

@nak3 @mgencur any comment before getting this in?

[skonto]

@matzew any comment? Otherwise will unhold.

[matzew]

/unhold

looks like for next release things are more smooth 🙏

thanks for the questions here, @pierDipi and the details, @skonto

[mgencur]

It would be great to setup image mirroring in https://github.com/openshift/release/blob/master/core-services/image-mirroring/knative/mapping_knative_v1_3_quay