Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove --cluster-name flag #2351

Merged
merged 1 commit into from
Feb 29, 2024
Merged

Remove --cluster-name flag #2351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/in-cluster-test-scripts/eks-tunnel.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ set -x
set -e

# Install Cilium
# We can't get rid of --cluster-name until we fix https://github.com/cilium/cilium-cli/issues/1347.
cilium install \
--version "${CILIUM_VERSION}" \
--cluster-name "${CLUSTER_NAME}" \
--set cluster.name="${CLUSTER_NAME}" \
--wait=false \
--set bpf.monitorAggregation=none \
--datapath-mode=tunnel \
Expand Down
2 changes: 1 addition & 1 deletion .github/in-cluster-test-scripts/eks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set -e
# Install Cilium
cilium install \
--version "${CILIUM_VERSION}" \
--cluster-name "${CLUSTER_NAME}" \
--set cluster.name="${CLUSTER_NAME}" \
--wait=false \
--set loadBalancer.l7.backend=envoy \
--set tls.secretsBackend=k8s \
Expand Down
3 changes: 1 addition & 2 deletions .github/in-cluster-test-scripts/external-workloads-install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ set -x
set -e

# Install Cilium in cluster
# We can't get rid of --cluster-name until we fix https://github.com/cilium/cilium-cli/issues/1347.
cilium install \
--version "${CILIUM_VERSION}" \
--cluster-name "${CLUSTER_NAME}" \
--set cluster.name="${CLUSTER_NAME}" \
--set bpf.monitorAggregation=none \
--datapath-mode=tunnel \
--set kubeProxyReplacement=strict \
Expand Down
3 changes: 1 addition & 2 deletions .github/in-cluster-test-scripts/gke.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ set -x
set -e

# Install Cilium
# We can't get rid of --cluster-name until we fix https://github.com/cilium/cilium-cli/issues/1347.
cilium install \
--version "${CILIUM_VERSION}" \
--cluster-name "${CLUSTER_NAME}" \
--set cluster.name="${CLUSTER_NAME}" \
--set bpf.monitorAggregation=none \
--set ipv4NativeRoutingCIDR="${CLUSTER_CIDR}" \
--set loadBalancer.l7.backend=envoy \
Expand Down
5 changes: 2 additions & 3 deletions .github/in-cluster-test-scripts/multicluster.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ CONTEXT1=$(kubectl config view | grep "${CLUSTER_NAME_1}" | head -1 | awk '{prin
CONTEXT2=$(kubectl config view | grep "${CLUSTER_NAME_2}" | head -1 | awk '{print $2}')

# Install Cilium in cluster1
# We can't get rid of --cluster-name until we fix https://github.com/cilium/cilium-cli/issues/1347.
cilium install \
--version "${CILIUM_VERSION}" \
--context "${CONTEXT1}" \
--set loadBalancer.l7.backend=envoy \
--set tls.secretsBackend=k8s \
--cluster-name "${CLUSTER_NAME_1}" \
--set cluster.name="${CLUSTER_NAME_1}" \
--set cluster.id=1 \
--set bpf.monitorAggregation=none \
--set ipv4NativeRoutingCIDR=10.0.0.0/9
Expand All @@ -32,7 +31,7 @@ cilium install \
--context "${CONTEXT2}" \
--set loadBalancer.l7.backend=envoy \
--set tls.secretsBackend=k8s \
--cluster-name "${CLUSTER_NAME_2}" \
--set cluster.name="${CLUSTER_NAME_2}" \
--set cluster.id=2 \
--set bpf.monitorAggregation=none \
--set ipv4NativeRoutingCIDR=10.0.0.0/9
Expand Down
2 changes: 1 addition & 1 deletion clustermesh/clustermesh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (k *K8sClusterMesh) GetClusterConfig(ctx context.Context) error {
k.clusterName = clusterName

if clusterID == "0" || clusterName == "default" {
k.Log("⚠️ Cluster not configured for clustermesh, use '--cluster-id' and '--cluster-name' with 'cilium install'. External workloads may still be configured.")
k.Log("⚠️ Cluster not configured for clustermesh, use '--set cluster.id' and '--set cluster.name' with 'cilium install'. External workloads may still be configured.")
}

return nil
Expand Down
1 change: 0 additions & 1 deletion install/autodetect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ func (k *K8sInstaller) autodetectAndValidate(ctx context.Context, helmValues map

k.Log("ℹ️ Using Cilium version %s", k.chartVersion)

// "cluster.name" Helm value takes precedence over --cluster-name flag.
clusterName := getClusterName(helmValues)
if clusterName != "" {
k.params.ClusterName = clusterName
Expand Down
2 changes: 0 additions & 2 deletions install/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ func (k *K8sInstaller) getHelmValues() (map[string]interface{}, error) {
}
}

// TODO: remove when removing "cluster-name" flag (marked as deprecated),
// kept for backwards compatibility
if k.params.ClusterName != "" {
helmMapOpts["cluster.name"] = k.params.ClusterName
}
Comment on lines -116 to 118
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can't remove this block because we need to propagate the cluster name that gets auto-detected in https://github.com/cilium/cilium-cli/blob/main/install/autodetect.go#L150

Expand Down
3 changes: 0 additions & 3 deletions internal/cli/cmd/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ import (

// addCommonInstallFlags adds install command flags that are shared between classic and helm mode.
func addCommonInstallFlags(cmd *cobra.Command, params *install.Parameters) {
// We can't get rid of --cluster-name until we fix https://github.com/cilium/cilium-cli/issues/1347.
cmd.Flags().StringVar(&params.ClusterName, "cluster-name", "", "Name of the cluster")
cmd.Flags().MarkDeprecated("cluster-name", "This can now be overridden via `--set` (Helm value: `cluster.name`).")
cmd.Flags().StringVar(&params.Version, "version", defaults.Version, "Cilium version to install")
cmd.Flags().StringVar(&params.DatapathMode, "datapath-mode", "", "Datapath mode to use { tunnel | native | aws-eni | gke | azure | aks-byocni } (default: autodetected).")
cmd.Flags().BoolVar(&params.ListVersions, "list-versions", false, "List all the available versions without actually installing")
Expand Down
Loading