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

Cilium yaml template formats AgentPrometheusPort into a UNICODE char instead of the port number #12717

Closed
michaelajr opened this issue Nov 12, 2021 · 1 comment · Fixed by #12721
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@michaelajr
Copy link

michaelajr commented Nov 12, 2021

/kind bug

1. What kops version are you running? The command kops version, will display
this information.

Version 1.21.0

2. What Kubernetes version are you running? kubectl version will print the
version if a cluster is running or provide the Kubernetes version specified as
a kops flag.

Client Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.4", GitCommit:"3cce4a82b44f032d0cd1a1790e6d2f5a55d20aae", GitTreeState:"clean", BuildDate:"2021-08-11T18:16:05Z", GoVersion:"go1.16.7", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.4", GitCommit:"3cce4a82b44f032d0cd1a1790e6d2f5a55d20aae", GitTreeState:"clean", BuildDate:"2021-08-11T18:10:22Z", GoVersion:"go1.16.7", Compiler:"gc", Platform:"linux/amd64"}

3. What cloud provider are you using?

AWS

4. What commands did you run? What is the simplest way to reproduce this issue?

kops create -f
kops update cluster

5. What happened after the commands executed?

The cluster comes up, but all the cilium pods have a UNICODE char as the value of the prometheus port annotation.

kubectl get pods -n kube-system cilium-cgvt7 -o yaml | less
apiVersion: v1
kind: Pod
metadata:
  annotations:
    kubernetes.io/psp: kube-system
    prometheus.io/port: 
    prometheus.io/scrape: "true"
    scheduler.alpha.kubernetes.io/critical-pod: ""
  creationTimestamp: "2021-11-12T00:21:47Z"
  generateName: cilium

Depending on your terminal encoding you might see this.

apiVersion: v1
kind: Pod
metadata:
  annotations:
    kubernetes.io/psp: kube-system
    prometheus.io/port: <E2><8E><82>
    prometheus.io/scrape: "true"
    scheduler.alpha.kubernetes.io/critical-pod: ""
  creationTimestamp: "2021-11-12T00:21:47Z"
  generateName: cilium-

The port kops uses by default is 9090. Which happens to be the UTF-16 decimal value for the character (https://www.fileformat.info/info/unicode/char/2382/index.htm). Note that <E2><8E><82> is the UTF-8 hex for .

How did this happen? The cilium template uses printf and the %q token which converts the value to a char not a string.

prometheus.io/port: {{ printf "%q" .AgentPrometheusPort }}

The line should use the decimal token and pipe to quote.

prometheus.io/port: {{ printf "%d" .AgentPrometheusPort | quote }}

This is causing serious issues with nri-prometheus (New Relic's prometheus scrape solution).

time="2021-11-12T00:50:00Z" level=error msg="couldn't parse pod url, skipping: http://10.157.43.174:⎂/metrics" component=KubernetesAPI error="parse \"http://10.157.43.174:⎂/metrics\": invalid port \":⎂\" after host" pod=cilium-cgvt7

This will also prevent people from using an in-cluster Prometheus looking for annotations. While a work around for the in-cluster Prometheus is to drop a PodMonitor - this is not doable for nri-prometheus which relies solely on the Prometheus annotations.

6. What did you expect to happen?

I expected to have the port number translated correctly in the Cilium manifest.

7. Please provide your cluster manifest. Execute
kops get --name my.example.com -o yaml to display your cluster manifest.
You may want to remove your cluster name and other sensitive information.

# pertinent part
spec:
  networking:
    cilium:
      enableRemoteNodeIdentity: true
      enablePrometheusMetrics: true

8. Please run the commands with most verbose logging by adding the -v 10 flag.
Paste the logs into this report, or in a gist and provide the gist link here.

N/A

9. Anything else do we need to know?

Nope.

@zhengtianbao
Copy link
Member

Thank you for spotting it, I have made a PR to solve this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants