Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: update cluster-proportional-autoscaler path at MCR #3091

Merged
merged 5 commits into from Apr 20, 2020

Conversation

mboersma
Copy link
Member

@mboersma mboersma commented Apr 17, 2020

Reason for Change:
The cluster-proportional-autoscaler image reference at mcr.microsoft.com was incorrect. Now that MCR is the default, this could cause calico cluster deployments to fail.

Issue Fixed:
Fixes #3079

Requirements:

Notes:
I did the before and after tests locally to confirm this fixes things. This probably should have been caught somewhere in Jenkins e2e, however. I will follow up.

@codecov
Copy link

codecov bot commented Apr 17, 2020

Codecov Report

Merging #3091 into master will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3091      +/-   ##
==========================================
- Coverage   70.65%   70.62%   -0.03%     
==========================================
  Files         147      147              
  Lines       25407    25407              
==========================================
- Hits        17951    17944       -7     
- Misses       6333     6341       +8     
+ Partials     1123     1122       -1     
Impacted Files Coverage Δ
pkg/api/k8s_versions.go 100.00% <ø> (ø)
pkg/api/addons.go 97.73% <100.00%> (ø)
pkg/engine/templates_generated.go 35.35% <0.00%> (-0.31%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 543083c...433008f. Read the comment docs.

@@ -19,7 +19,7 @@ const (
calicoCNIImageReference string = "cni:v3.8.0"
calicoNodeImageReference string = "node:v3.8.0"
calicoPod2DaemonImageReference string = "pod2daemon-flexvol:v3.8.0"
calicoClusterProportionalAutoscalerImageReference string = "cluster-proportional-autoscaler-amd64:1.1.2-r2"
calicoClusterProportionalAutoscalerImageReference string = "oss/kubernetes/autoscaler/cluster-proportional-autoscaler:1.1.2-r2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to turn this into the full mcr URL, and get rid of the kubernetesImageBase prefix dep. Basically we don't need to support both GCR and MCR types here.

Also, not to add scope creep, but is it worth seeing if a newer version of this works w/ the current calico spec?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth seeing if a newer version of this works...?

I think so. I added a commit to put all three versions of C-P-A in the VHD but ideally we would all use 1.7.1 (or the 1.7.1-hotfix?). I'll try that next.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since this is a new regression in v0.49.0, let's defer the version bumps to a separate PR.

@andyliuliming
Copy link
Member

/lgtm

Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@mboersma mboersma merged commit b8a1018 into Azure:master Apr 20, 2020
@mboersma mboersma deleted the fix-cluster-prop-auto branch April 20, 2020 17:56
alexeldeib pushed a commit to alexeldeib/aks-engine that referenced this pull request Apr 21, 2020
* fix: update cluster-proportional-autoscaler path at MCR

* chore: add other cluster-prop-auto versions to Linux VHD

* fix: remove DNSAutoscalerAddonName after rebase fail

* chore: remove unreferenced c-a-p version from Linux VHD

* refactor: remove kubernetesImageBase prefix from calico c-p-a
@andyliuliming
Copy link
Member

Hi @jackfrancis @mboersma when will this been release? :)

@jackfrancis
Copy link
Member

@andyliuliming we are in the process of validating master for a v0.50.0 release by end of week

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cluster-proportional-autoscaler-amd64:1.1.2-r2 fails to pull from mcr.microsoft.com ( ImagePullBackOff )
3 participants