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

fix(provider/kubernetes): Do not raise exception for named port in tcpSocket probes #2619

Merged
merged 3 commits into from
May 16, 2018

Conversation

dtan4
Copy link
Contributor

@dtan4 dtan4 commented May 10, 2018

If named port is given as tcpSocket liveness/readiness probe, do not raise any exception and warn that only.

When a Kubernetes resource uses named port in its tcpSocket liveness/readiness probe, clouddriver raises java.lang.NumberFormatException. Clouddriver tries to parse port name redis as integer.

apiVersion: v1
items:
- apiVersion: apps/v1
  kind: StatefulSet
  metadata:
   ...
  spec:
    ...
    serviceName: redis
    template:
      metadata:
        name: redis
      spec:
        containers:
        - args:
          ...
          livenessProbe:
            failureThreshold: 3
            initialDelaySeconds: 30
            periodSeconds: 10
            successThreshold: 1
            tcpSocket:
              port: redis # <================================
            timeoutSeconds: 5
          name: redis
          ports:
          - containerPort: 6379
            name: redis
            protocol: TCP
java.lang.NumberFormatException: For input string: "redis"
        at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65) ~[na:1.8.0_151]
        at java.lang.Integer.parseInt(Integer.java:580) ~[na:1.8.0_151]
        at java.lang.Integer.valueOf(Integer.java:766) ~[na:1.8.0_151]
        at org.codehaus.groovy.runtime.StringGroovyMethods.toInteger(StringGroovyMethods.java:3312) ~[groovy-all-2.4.12.jar:2.4.12]
        at org.codehaus.groovy.runtime.dgm$1160.invoke(Unknown Source) ~[groovy-all-2.4.12.jar:2.4.12]
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoMetaMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:274) ~[groovy-all-2.4.12.jar:2.4.12]
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:56) ~[groovy-all-2.4.12.jar:2.4.12]
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:117) ~[groovy-all-2.4.12.jar:2.4.12]
        at com.netflix.spinnaker.clouddriver.kubernetes.v1.api.KubernetesClientApiConverter.fromTcpSocketAction(KubernetesClientApiConverter.groovy:315) ~[clouddriver-kubernetes-1.775.0-SNAPSHOT.jar:1.775.0-SNAPSHOT]
        at com.netflix.spinnaker.clouddriver.kubernetes.v1.api.KubernetesClientApiConverter$fromTcpSocketAction$0.callStatic(Unknown Source) ~[na:na]
        at com.netflix.spinnaker.clouddriver.kubernetes.v1.api.KubernetesClientApiConverter.fromV1Probe(KubernetesClientApiConverter.groovy:338) ~[clouddriver-kubernetes-1.775.0-SNAPSHOT.jar:1.775.0-SNAPSHOT]
        at com.netflix.spinnaker.clouddriver.kubernetes.v1.api.KubernetesClientApiConverter$fromV1Probe.callStatic(Unknown Source) ~[na:na]
        at com.netflix.spinnaker.clouddriver.kubernetes.v1.api.KubernetesClientApiConverter.fromContainer(KubernetesClientApiConverter.groovy:197) ~[clouddriver-kubernetes-1.775.0-SNAPSHOT.jar:1.775.0-SNAPSHOT]

Clouddriver does the same behavior for named port in httpGet probe. I followed the implementation.

@@ -354,7 +354,11 @@ class KubernetesClientApiConverter {
}

def kubernetesTcpSocketAction = new KubernetesTcpSocketAction()
kubernetesTcpSocketAction.port = tcpSocket.port.toInteger() ?: 0
try {
kubernetesTcpSocketAction.port = tcpSocket.port.toInteger() ?: 0
Copy link
Member

Choose a reason for hiding this comment

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

What happens if you try to use the Spinnaker UI to clone a server group that uses a named port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried the behavior with the below steps,

  1. Deploy ReplicaSet (new server group) with named ports
  2. Clone created server group to new one

then confirmed that all named ports were cloned properly with name.

Copy link
Member

Choose a reason for hiding this comment

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

Sweet!

@lwander
Copy link
Member

lwander commented May 16, 2018

Closes spinnaker/spinnaker#1210

@lwander lwander merged commit 828c08d into spinnaker:master May 16, 2018
@dtan4 dtan4 deleted the k8s-v1-probe-tcp-named-port branch May 17, 2018 08:12
lwander added a commit to lwander/clouddriver that referenced this pull request Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants