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.

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

Forced to change k8s cluster name as per regex. #1871

Closed
rahulk789 opened this issue Jul 27, 2023 · 6 comments · Fixed by #1877
Closed

Forced to change k8s cluster name as per regex. #1871

rahulk789 opened this issue Jul 27, 2023 · 6 comments · Fixed by #1877
Labels
kind/bug Something isn't working

Comments

@rahulk789
Copy link
Contributor

Bug report

Hey, just trying to get the cilium ingress controller on my k8s and I got this:

$ cilium install --version 1.13.4 --set kubeProxyReplacement=true --set ingressController.enabled=true --set ingressController.loadbalancerMode=dedicated
🔮 Auto-detected Kubernetes kind: EKS
ℹ️ Using Cilium version 1.13.4
🔮 Auto-detected cluster name: arn:....../EKS-Cluster
🔮 Auto-detected datapath mode: aws-eni
❌ Cluster name "/EKS-Cluster" is not valid, must match regular expression: ^[a-z0-9]([-a-z0-9]*[a-z0-9])$
↩️ Rolling back installation...
Error: Unable to install Cilium: invalid cluster name

I really find it inconvenient to change the name of an already running eks cluster binded to other configurations just for the sake of installing the cilium ingress controller. If this is considered as an valid issue, im willing to fix it myself. Thanks.

@rahulk789 rahulk789 added the kind/bug Something isn't working label Jul 27, 2023
@rahulk789
Copy link
Contributor Author

Relevant comments from Amit in slack:

Hi, few things here.
Firstly you cannot add / to the cluster-name this is a limitation from EKS
eksctl create cluster -f ./eks-config_ipv4.yaml
2023-07-27 20:28:01 [▶] role ARN for the current session is "arn:aws:iam::XXXXXXXXXXXXXXXXXXX"
Error: validation for /EKS-Cluster failed, name must satisfy regular expression pattern: [a-zA-Z][-a-zA-Z0-9]*
Yes, if you try to use cilium install --helm-set cluster.name=EKS-Cluster that also doesn’t work and we hit the issue as you are seeing
🔮 Auto-detected Kubernetes kind: EKS
ℹ️ Using Cilium version 1.13.3
🔮 Auto-detected cluster name: EKS-Cluster-eu-west-2-eksctl-io
🔮 Auto-detected datapath mode: aws-eni
❌ Cluster name "EKS-Cluster-eu-west-2-eksctl-io" is not valid, must match regular expression: ^a-z0-9$
↩️ Rolling back installation...
Good part is this
If you try to pass the cluster name via helm that works cluster.name
helm install cilium cilium/cilium --version 1.13.4
--namespace kube-system
--set eni.enabled=true
--set ipam.mode=eni
--set egressMasqueradeInterfaces=eth0
--set tunnel=disabled
--set cluster.name=EKS-Cluster
NAME: cilium
LAST DEPLOYED: Thu Jul 27 21:40:45 2023
NAMESPACE: kube-system
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
You have successfully installed Cilium with Hubble.

Your release version is 1.13.4.

For any further help, visit https://docs.cilium.io/en/v1.13/gettinghelp
cilium config view | grep cluster-name
cluster-name EKS-Cluster

@joestringer
Copy link
Member

x Cluster name "/EKS-Cluster" is not valid, must match regular expression: ^a-z0-9$

I think this is the key, the regex there is only including lower-case letters. Given that the helm option accepts uppercase as well, this might just be an oversight.

@rahulk789
Copy link
Contributor Author

should I make a pr? @joestringer

@joestringer
Copy link
Member

Sure, that would be awesome!

@amitmavgupta
Copy link

Thanks @rahulk789

@asauber
Copy link
Member

asauber commented Aug 1, 2023

This codepath is used only if you are in the CLI "classic" mode (environment CILIUM_CLI_MODE=classic)

If you're using the latest version of the CLI, you may want to try Helm mode. It uses an embedded Helm client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants