cluster: Allow deploying controller on external cluster #255
Conversation
oshoval
changed the title
oshoval
changed the title
|
addressed comments, ptal |
hack/e2e-kind-cluster-setup.sh
Outdated
| } | ||
|
|
||
| cleanup() { | ||
| rm -rf multus-cni | ||
| git checkout -- manifests/ | ||
| } | ||
|
|
||
| install_multus() { | ||
| if [[ $MULTUS_VERSION == "latest" ]]; then | ||
| echo "error: MULTUS_VERSION cant be latest, when using non default CLUSTER_TYPE $CLUSTER_TYPE" |
There was a problem hiding this comment.
maybe be explicit about what cluster type you need to use here.
Also, why is this limitation in place ?
There was a problem hiding this comment.
We reach here only when CLUSTER_TYPE != kind which is the default value.
So the CLUSTER_TYPE is already set correctly, what is not set is MULTUS_VERSION and this what the comment says.
The reason we have it, is that for default CLUSTER_TYPE we use setup_cluster.sh [1] that creates a kind cluster and then build + deploy latest multus-cni.
Here we dont want to create a new cluster (which is the answer for the question), so for now we support deploying an official multus release if desired, by providing its MULTUS_VERSION.
There isn't such thing
https://raw.githubusercontent.com/k8snetworkplumbingwg/multus-cni/latest/deployments/multus-daemonset-thick.yml so this assert the value of the version is valid.
If we would need to also support latest freshly built multus-cni in the future we can but it isnt as straight forward.
This PR is MVP.
PR desc has suggested exports.
There was a problem hiding this comment.
OK, so change two things in the PR:
- stop saying "default cluster type". Start saying kind cluster. Be explicit.
- explain why: when kind is used, it already has the latest multus deployed.
hack/e2e-kind-cluster-setup.sh
Outdated
| } | ||
|
|
||
| cleanup() { | ||
| rm -rf multus-cni | ||
| git checkout -- manifests/ | ||
| } | ||
|
|
||
| install_multus() { | ||
| if [[ $MULTUS_VERSION == "latest" ]]; then | ||
| echo "error: MULTUS_VERSION cant be latest, when using non default CLUSTER_TYPE $CLUSTER_TYPE" |
There was a problem hiding this comment.
| echo "error: MULTUS_VERSION cant be latest, when using non default CLUSTER_TYPE $CLUSTER_TYPE" | |
| echo "error: kind cluster is already shipping latest multus (built from src). No point installing it |
There was a problem hiding this comment.
it is not accurate, we reach here only if CLUSTER_TYPE isn't kind
and then we can only use snapshot and not latest
please see https://github.com/k8snetworkplumbingwg/multus-dynamic-networks-controller/pull/255/files#r1793352623
hack/e2e-kind-cluster-setup.sh
Outdated
| } | ||
|
|
||
| cleanup() { | ||
| rm -rf multus-cni | ||
| git checkout -- manifests/ | ||
| } | ||
|
|
||
| install_multus() { |
There was a problem hiding this comment.
so this function is only invoked on kind based clusters, right ?
Let's rename it to something that hints this is used to request a particular multus snapshot / release.
Does that make sense to you ?
There was a problem hiding this comment.
the other way around, only on non kind
https://github.com/k8snetworkplumbingwg/multus-dynamic-networks-controller/pull/255/files#diff-e4e4a234613c45b53133df21b3af7637ca37af944aac581dbf765dead1927808R60
see L65
ye i can rename it
Allows to deploy Multus dynamic networks controller on external provider. Deploys fixed multus release (doesnt build latest as this repo usually do). Usage: export CLUSTER_TYPE=external export KUBECONFIG=<kubeconfig> export IMAGE_REGISTRY=quay.io/<your_quay> export MULTUS_VERSION=v4.1.1 If CRIO is used, use export CRI=crio (works also for non external clusters). If You don't want to deploy multus at all use (you must deploy it yourself as the controller depends on it) export SKIP_MULTUS_DEPLOYMENT=true (supported only for external clusters) Signed-off-by: Or Shoval <[email protected]>
|
tried to address comments, according the latest discussion, please take a look |
|
yes, for example kubevirtci has CRIO |
What this PR does / why we need it:
Allows to deploy Multus dynamic networks controller on external provider.
Deploys official multus release (doesnt build latest as this repo usually do).
Usage example:
If CRIO is used, use
export CRI=crio(works also for non external clusters).Note kubevirtci clusters require it.
If You don't want to deploy multus at all use (you must deploy it yourself as the controller depends on it)
export SKIP_MULTUS_DEPLOYMENT=true(supported only for external clusters)Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #252
Special notes for your reviewer (optional):
Can be improved later according needs, for example to support deploying latest Multus,
or to support local registries.
File name is bit tricky as it has the word kind in its name, we can consider adding a new file instead
with those changes for external providers, and source the common functions.