cloud-provider-azure 1.23.2 CA dependency is broken #4875

MaciekPytel · 2022-05-10T12:18:42Z

CA has a dependency on cloud-provider-azure 1.23.2, which contains a file with mismatched quotes that breaks CA tests.

Interestingly the file in CA vendor directory doesn't have the same bug. The problematic file was first introduced to CA vendor in #4689, without the bug. The only way I can think for how this would be possible is if the problem was manually fixed directly in the vendor directory (but the PR doesn't even mention anything like this).

Can we update the cloud-provider-azure dependency to a version with the fix? Currently any attempt to update vendor fails, because of this bug being introduced by the update.
- This is currently blocking 1.24 release. Is this version bump something we can safely perform last minute? The alternative is to carry the fix in the vendor (by manually removing the problematic file from vendor update commit), but this comes with its own problems. For example anyone importing CA 1.24.0 is likely to run into the very same issue (as they would get the transitive dependency, but not the modified version in CA vendor/).
If my guess above was correct, can we make sure not to fix any bugs or make any code changes directly in CA vendor directory going forward?

@gandhipr @marwanad @feiskyer

x13n · 2022-05-10T14:32:30Z

I think the long term solution (aka point 3) should be to just stop using vendor, just created #4878 for this.

gandhipr · 2022-05-10T17:13:38Z

I agree. Point 3 should be stopped. Apologies for #4689 <#4689>. I remember creating a change in cloud-provider-azure. In my opinion, we should update cloud-provider-azure dependency to a version with a fix and verify that tests don't fail. The alternative is to carry the fix in the vendor would be a quick fix but will cause other issues as mentioned above.

…

On Tue, May 10, 2022 at 7:32 AM Daniel Kłobuszewski < ***@***.***> wrote: I think the long term solution (aka point 3) should be to just stop using vendor, just created #4878 <#4878> for this. — Reply to this email directly, view it on GitHub <#4875 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ARQAMBUZ5C4ZOVV2MS33RZLVJJXQVANCNFSM5VRM2UXQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

marwanad · 2022-05-10T18:38:15Z

The cloudprovider version should be bumped to v1.24.x which contains this fix. The fix seems to be in 1.23.11 as well.

MaciekPytel · 2022-05-11T12:20:43Z

Bumping cloudprovider to either 1.23.11 or 1.24 causes compilation error in CA azure cloudprovider. I'm guessing they shouldn't be very hard to fix, but I don't want to make last minute changes in cloudprovider I don't know and have no way of testing.

I'm leaning towards manually fixing this in vendor/, listing the import problem as a know issue and fixing it in a follow-up patch release. Unless you want to open a PR bumping the version of cloud-provider-azure and you have enough confidence that those changes are safe enough to include in 1.24.0?

Manually removed the update to vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/doc.go as per discussion in kubernetes#4875.

MaciekPytel · 2022-05-11T13:44:24Z

I've updated #4871 to carry the fix in vendor. If there are no comments against this approach and / or alternative PRs with version update for Azure by tomorrow, I'd like to merge it tomorrow to unblock 1.24.0 release.

gandhipr · 2022-05-11T15:25:10Z

Okay, sounds good. I'll work on updating the vendor version in parallel. Thank you.

…

On Wed, May 11, 2022 at 6:44 AM MaciekPytel ***@***.***> wrote: I've updated #4871 <#4871> to carry the fix in vendor. If there are no comments against this approach and / or alternative PRs with version update for Azure by tomorrow, I'd like to merge it tomorrow to unblock 1.24.0 release. — Reply to this email directly, view it on GitHub <#4875 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ARQAMBWV63X766SMBO6AM73VJO2UHANCNFSM5VRM2UXQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

gandhipr · 2022-05-11T19:14:41Z

I created this PR: #4883 fixing version issue in CA. On Wed, May 11, 2022 at 8:24 AM Prachi Gandhi ***@***.***> wrote:

…

Okay, sounds good. I'll work on updating the vendor version in parallel. Thank you. On Wed, May 11, 2022 at 6:44 AM MaciekPytel ***@***.***> wrote: > I've updated #4871 <#4871> > to carry the fix in vendor. If there are no comments against this approach > and / or alternative PRs with version update for Azure by tomorrow, I'd > like to merge it tomorrow to unblock 1.24.0 release. > > — > Reply to this email directly, view it on GitHub > <#4875 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ARQAMBWV63X766SMBO6AM73VJO2UHANCNFSM5VRM2UXQ> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >

gandhipr · 2022-05-12T05:03:10Z

cherry-picking above PR to 1.22 and 1.23. I'll push these changes onto 1.24 after release? On Wed, May 11, 2022 at 12:14 PM Prachi Gandhi ***@***.***> wrote:

…

I created this PR: #4883 fixing version issue in CA. On Wed, May 11, 2022 at 8:24 AM Prachi Gandhi ***@***.***> wrote: > Okay, sounds good. I'll work on updating the vendor version in parallel. > Thank you. > > On Wed, May 11, 2022 at 6:44 AM MaciekPytel ***@***.***> > wrote: > >> I've updated #4871 <#4871> >> to carry the fix in vendor. If there are no comments against this approach >> and / or alternative PRs with version update for Azure by tomorrow, I'd >> like to merge it tomorrow to unblock 1.24.0 release. >> >> — >> Reply to this email directly, view it on GitHub >> <#4875 (comment)>, >> or unsubscribe >> <https://github.com/notifications/unsubscribe-auth/ARQAMBWV63X766SMBO6AM73VJO2UHANCNFSM5VRM2UXQ> >> . >> You are receiving this because you were mentioned.Message ID: >> ***@***.***> >> >

Manually removed the update to vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/doc.go as per discussion in kubernetes#4875. # Conflicts: # vertical-pod-autoscaler/vendor/k8s.io/api/certificates/v1/register.go # vertical-pod-autoscaler/vendor/k8s.io/api/events/v1beta1/zz_generated.prerelease-lifecycle.go # vertical-pod-autoscaler/vendor/k8s.io/api/flowcontrol/v1beta1/doc.go # vertical-pod-autoscaler/vendor/k8s.io/api/node/v1/zz_generated.deepcopy.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/applyconfigurations/batch/v1/job.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/applyconfigurations/core/v1/podip.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/applyconfigurations/extensions/v1beta1/ipblock.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/applyconfigurations/extensions/v1beta1/networkpolicystatus.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/informers/apiserverinternal/v1alpha1/storageversion.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/informers/policy/v1/interface.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/kubernetes/typed/apiserverinternal/v1alpha1/apiserverinternal_client.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/kubernetes/typed/apiserverinternal/v1alpha1/doc.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/kubernetes/typed/apiserverinternal/v1alpha1/fake/doc.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/kubernetes/typed/autoscaling/v2/generated_expansion.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/kubernetes/typed/certificates/v1/fake/fake_certificates_client.go # vertical-pod-autoscaler/vendor/k8s.io/client-go/listers/node/v1/expansion_generated.go

Manually removed the update to vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/doc.go as per discussion in kubernetes#4875.

k8s-triage-robot · 2022-08-10T06:01:34Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

feiskyer · 2022-08-11T02:40:05Z

this should be fixed already
/close

k8s-ci-robot · 2022-08-11T02:40:21Z

@feiskyer: Closing this issue.

In response to this:

this should be fixed already
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

MaciekPytel added kind/bug Categorizes issue or PR as related to a bug. area/cluster-autoscaler labels May 10, 2022

MaciekPytel mentioned this issue May 10, 2022

Update vendor to Kubernetes 1.24.0 #4871

Merged

x13n mentioned this issue May 10, 2022

Abandon checked-in vendor directory in Cluster Autoscaler #4878

Closed

MaciekPytel added a commit to MaciekPytel/autoscaler that referenced this issue May 11, 2022

Update dependencies for Cluster Autoscaler 1.24.0

658e50d

Manually removed the update to vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/doc.go as per discussion in kubernetes#4875.

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 10, 2022

k8s-ci-robot closed this as completed Aug 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cloud-provider-azure 1.23.2 CA dependency is broken #4875

cloud-provider-azure 1.23.2 CA dependency is broken #4875

MaciekPytel commented May 10, 2022 •

edited

Loading

x13n commented May 10, 2022

gandhipr commented May 10, 2022 via email

marwanad commented May 10, 2022 •

edited

Loading

MaciekPytel commented May 11, 2022

MaciekPytel commented May 11, 2022

gandhipr commented May 11, 2022 via email

gandhipr commented May 11, 2022 via email

gandhipr commented May 12, 2022 via email

k8s-triage-robot commented Aug 10, 2022

feiskyer commented Aug 11, 2022

k8s-ci-robot commented Aug 11, 2022

cloud-provider-azure 1.23.2 CA dependency is broken #4875

cloud-provider-azure 1.23.2 CA dependency is broken #4875

Comments

MaciekPytel commented May 10, 2022 • edited Loading

x13n commented May 10, 2022

gandhipr commented May 10, 2022 via email

marwanad commented May 10, 2022 • edited Loading

MaciekPytel commented May 11, 2022

MaciekPytel commented May 11, 2022

gandhipr commented May 11, 2022 via email

gandhipr commented May 11, 2022 via email

gandhipr commented May 12, 2022 via email

k8s-triage-robot commented Aug 10, 2022

feiskyer commented Aug 11, 2022

k8s-ci-robot commented Aug 11, 2022

MaciekPytel commented May 10, 2022 •

edited

Loading

marwanad commented May 10, 2022 •

edited

Loading