Install CNI network plugins only for specific CNIs #17162
Conversation
k8s-ci-robot
added
cncf-cla: yes
hakman
changed the title
k8s-ci-robot
added
the
do-not-merge/work-in-progress
| defaultCNIAssetAmd64K8s_27 = "https://storage.googleapis.com/k8s-artifacts-cni/release/v1.2.0/cni-plugins-linux-amd64-v1.2.0.tgz" | ||
| defaultCNIAssetArm64K8s_27 = "https://storage.googleapis.com/k8s-artifacts-cni/release/v1.2.0/cni-plugins-linux-arm64-v1.2.0.tgz" | ||
|
|
||
| defaultCNIAssetAmd64K8s_29 = "https://storage.googleapis.com/k8s-artifacts-cni/release/v1.3.0/cni-plugins-linux-amd64-v1.3.0.tgz" |
There was a problem hiding this comment.
We keep this is in WIP until we get some consensus about mirroring CNI to artifacts.k8s.io?
There was a problem hiding this comment.
Doesn't matter much, as this is for kOps 1.32, which is early alpha. For sure we will add mirrors for this.
There was a problem hiding this comment.
Also, please not that the top 3 use CNIs don't need those binaries, so even without mirrors, it should not be very demanding.
|
We don't want to generate assets for CNI plugins ? I don't see any change in |
We want, but did not have time for that yet. 😁 |
k8s-ci-robot
added
size/XXL
hakman
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/test all |
|
@rifelpet If you get a chance, could you take a look at this? Thanks! |
| if b.NodeupConfig.Networking.AmazonVPC == nil && | ||
| b.NodeupConfig.Networking.Calico == nil && | ||
| b.NodeupConfig.Networking.Cilium == nil { |
There was a problem hiding this comment.
can we reuse the new logic in pkg/apis/kops/model/instance_group.go ? that way we dont have to remember to update the same conditional in two places.
k8s-ci-robot
removed
the
lgtm
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rifelpet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| @@ -40,6 +40,12 @@ var wellKnownMirrors = []mirrorConfig{ | |||
| "https://github.com/kubernetes/kops/releases/download/v{kopsVersion}/", | |||
| }, | |||
| }, | |||
| { | |||
| Base: "https://storage.googleapis.com/k8s-artifacts-cni/release/", | |||
There was a problem hiding this comment.
shouldn't we invert this?
long term we'd like to eliminate this host, we have been exiting making kubernetes responsible for hosting third party assets, especially on non-rate-limited hosts that we pay for (And it seems the main use case to not just fetch from github is bypassing their rate limits)
There was a problem hiding this comment.
There was a problem hiding this comment.
Agreed, the main use case is bypassing the rate limits.
We started discussing inverting this today, but no conclusion yet. The initial intention was to keep this consistent and decide later which ones to change to the mirror as primary.
@justinsb any preference on how to approach this?
There was a problem hiding this comment.
Agreed, the main use case is bypassing the rate limits.
Yeah, the problem is this then makes us responsible for no rate limit, I think if users are pulling so much that github is blocking them, then they probably need to use their own mirror or something?
We have been rate limiting other hosts like registry.k8s.io, but that's not possible here without moving to a different host (at which point ... just use github / official downloads?)
/cc @ameukam @rifelpet @justinsb