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

Enhance the Ingress Addon #3099

Merged
merged 1 commit into from
Oct 4, 2018
Merged

Conversation

diazjf
Copy link
Contributor

@diazjf diazjf commented Aug 29, 2018

Adds a few updates to the Ingress Addon.

  • Updates Ingress-Controller Version to 0.19.0
  • Adds Service Account for Ingress-Controller
  • Adds Support for Prometheus
  • Fixes bug with TCP/UDP ConfigMaps not Loading
  • Adds more resource limits to default-backend
  • Use new ingress class name
  • Use app.kubernetes.io/xxxxxxxxxxx labels

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 29, 2018
@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 29, 2018
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

WIP: ClusterRoleBinding are not being created properly.

@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

@aledbf lmk what you think.

@diazjf diazjf force-pushed the update-ingress-addon branch 2 times, most recently from 16cfa76 to 6fd23a8 Compare August 29, 2018 20:03
@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

Got the RBAC working. Service Account being used.

$ kubectl get deploy -n kube-system nginx-ingress-controller -o yaml

apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  annotations:
    deployment.kubernetes.io/revision: "1"
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"extensions/v1beta1","kind":"Deployment","metadata":{"annotations":{},"labels":{"addonmanager.kubernetes.io/mode":"Reconcile","app.kubernetes.io/name":"nginx-ingress-controller","app.kubernetes.io/part-of":"kube-system"},"name":"nginx-ingress-controller","namespace":"kube-system"},"spec":{"replicas":1,"selector":{"matchLabels":{"addonmanager.kubernetes.io/mode":"Reconcile","app.kubernetes.io/name":"nginx-ingress-controller","app.kubernetes.io/part-of":"kube-system"}},"template":{"metadata":{"annotations":{"prometheus.io/port":"10254","prometheus.io/scrape":"true"},"labels":{"addonmanager.kubernetes.io/mode":"Reconcile","app.kubernetes.io/name":"nginx-ingress-controller","app.kubernetes.io/part-of":"kube-system"}},"spec":{"containers":[{"args":["/nginx-ingress-controller","--default-backend-service=$(POD_NAMESPACE)/default-http-backend","--configmap=$(POD_NAMESPACE)/nginx-load-balancer-conf","--tcp-services-configmap=$(POD_NAMESPACE)/tcp-services","--udp-services-configmap=$(POD_NAMESPACE)/udp-services","--annotations-prefix=nginx.ingress.kubernetes.io","--report-node-internal-ip-address"],"env":[{"name":"POD_NAME","valueFrom":{"fieldRef":{"fieldPath":"metadata.name"}}},{"name":"POD_NAMESPACE","valueFrom":{"fieldRef":{"fieldPath":"metadata.namespace"}}}],"image":"quay.io/kubernetes-ingress-controller/nginx-ingress-controller:0.18.0","imagePullPolicy":"IfNotPresent","livenessProbe":{"httpGet":{"path":"/healthz","port":10254,"scheme":"HTTP"},"initialDelaySeconds":10,"timeoutSeconds":1},"name":"nginx-ingress-controller","ports":[{"containerPort":80,"hostPort":80},{"containerPort":443,"hostPort":443},{"containerPort":18080,"hostPort":18080}],"readinessProbe":{"httpGet":{"path":"/healthz","port":10254,"scheme":"HTTP"}},"securityContext":{"capabilities":{"add":["NET_BIND_SERVICE"],"drop":["ALL"]},"runAsUser":33}}],"serviceAccountName":"nginx-ingress","terminationGracePeriodSeconds":60}}}}
  creationTimestamp: 2018-08-29T20:09:47Z
  generation: 1
  labels:
    addonmanager.kubernetes.io/mode: Reconcile
    app.kubernetes.io/name: nginx-ingress-controller
    app.kubernetes.io/part-of: kube-system
  name: nginx-ingress-controller
  namespace: kube-system
  resourceVersion: "592"
  selfLink: /apis/extensions/v1beta1/namespaces/kube-system/deployments/nginx-ingress-controller
  uid: 7a9a7316-abc7-11e8-9a2c-08002703cbee
spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      addonmanager.kubernetes.io/mode: Reconcile
      app.kubernetes.io/name: nginx-ingress-controller
      app.kubernetes.io/part-of: kube-system
  strategy:
    rollingUpdate:
      maxSurge: 1
      maxUnavailable: 1
    type: RollingUpdate
  template:
    metadata:
      annotations:
        prometheus.io/port: "10254"
        prometheus.io/scrape: "true"
      creationTimestamp: null
      labels:
        addonmanager.kubernetes.io/mode: Reconcile
        app.kubernetes.io/name: nginx-ingress-controller
        app.kubernetes.io/part-of: kube-system
    spec:
      containers:
      - args:
        - /nginx-ingress-controller
        - --default-backend-service=$(POD_NAMESPACE)/default-http-backend
        - --configmap=$(POD_NAMESPACE)/nginx-load-balancer-conf
        - --tcp-services-configmap=$(POD_NAMESPACE)/tcp-services
        - --udp-services-configmap=$(POD_NAMESPACE)/udp-services
        - --annotations-prefix=nginx.ingress.kubernetes.io
        - --report-node-internal-ip-address
        env:
        - name: POD_NAME
          valueFrom:
            fieldRef:
              apiVersion: v1
              fieldPath: metadata.name
        - name: POD_NAMESPACE
          valueFrom:
            fieldRef:
              apiVersion: v1
              fieldPath: metadata.namespace
        image: quay.io/kubernetes-ingress-controller/nginx-ingress-controller:0.18.0
        imagePullPolicy: IfNotPresent
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: /healthz
            port: 10254
            scheme: HTTP
          initialDelaySeconds: 10
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        name: nginx-ingress-controller
        ports:
        - containerPort: 80
          hostPort: 80
          protocol: TCP
        - containerPort: 443
          hostPort: 443
          protocol: TCP
        - containerPort: 18080
          hostPort: 18080
          protocol: TCP
        readinessProbe:
          failureThreshold: 3
          httpGet:
            path: /healthz
            port: 10254
            scheme: HTTP
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 1
        resources: {}
        securityContext:
          capabilities:
            add:
            - NET_BIND_SERVICE
            drop:
            - ALL
          runAsUser: 33
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: File
      dnsPolicy: ClusterFirst
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext: {}
      serviceAccount: nginx-ingress
      serviceAccountName: nginx-ingress
      terminationGracePeriodSeconds: 60
status:
  availableReplicas: 1
  conditions:
  - lastTransitionTime: 2018-08-29T20:09:47Z
    lastUpdateTime: 2018-08-29T20:09:47Z
    message: Deployment has minimum availability.
    reason: MinimumReplicasAvailable
    status: "True"
    type: Available
  - lastTransitionTime: 2018-08-29T20:09:47Z
    lastUpdateTime: 2018-08-29T20:10:14Z
    message: ReplicaSet "nginx-ingress-controller-64bdbc9dc7" has successfully progressed.
    reason: NewReplicaSetAvailable
    status: "True"
    type: Progressing
  observedGeneration: 1
  readyReplicas: 1
  replicas: 1
  updatedReplicas: 1

$ kubectl logs -n kube-system nginx-ingress-controller-64bdbc9dc7-k4fvq

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:    0.18.0
  Build:      git-7b20058
  Repository: https://github.com/kubernetes/ingress-nginx.git
-------------------------------------------------------------------------------

nginx version: nginx/1.15.2
W0829 20:10:10.934184       6 client_config.go:552] Neither --kubeconfig nor --master was specified.  Using the inClusterConfig.  This might not work.
I0829 20:10:10.934573       6 main.go:191] Creating API client for https://10.96.0.1:443
I0829 20:10:10.947612       6 main.go:235] Running in Kubernetes cluster version v1.10 (v1.10.0) - git (clean) commit fc32d2f3698e36b93322a3465f63a14e9f0eaead - platform linux/amd64
I0829 20:10:10.949564       6 main.go:100] Validated kube-system/default-http-backend as the default backend.
I0829 20:10:11.313663       6 nginx.go:255] Starting NGINX Ingress controller
I0829 20:10:11.332586       6 event.go:221] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"kube-system", Name:"nginx-load-balancer-conf", UID:"7a437e49-abc7-11e8-9a2c-08002703cbee", APIVersion:"v1", ResourceVersion:"500", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap kube-system/nginx-load-balancer-conf
I0829 20:10:11.335407       6 event.go:221] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"kube-system", Name:"tcp-services", UID:"7a443475-abc7-11e8-9a2c-08002703cbee", APIVersion:"v1", ResourceVersion:"501", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap kube-system/tcp-services
I0829 20:10:11.335583       6 event.go:221] Event(v1.ObjectReference{Kind:"ConfigMap", Namespace:"kube-system", Name:"udp-services", UID:"7a45a7f9-abc7-11e8-9a2c-08002703cbee", APIVersion:"v1", ResourceVersion:"502", FieldPath:""}): type: 'Normal' reason: 'CREATE' ConfigMap kube-system/udp-services
I0829 20:10:12.514974       6 nginx.go:276] Starting NGINX process
I0829 20:10:12.515388       6 leaderelection.go:185] attempting to acquire leader lease  kube-system/ingress-controller-leader-nginx...
I0829 20:10:12.515931       6 controller.go:169] Configuration changes detected, backend reload required.
I0829 20:10:12.523604       6 leaderelection.go:194] successfully acquired lease kube-system/ingress-controller-leader-nginx
I0829 20:10:12.523774       6 status.go:197] new leader elected: nginx-ingress-controller-64bdbc9dc7-k4fvq
I0829 20:10:12.657967       6 controller.go:185] Backend successfully reloaded.
I0829 20:10:12.659378       6 controller.go:195] Initial synchronization of the NGINX configuration.
I0829 20:10:13.662364       6 controller.go:202] Dynamic reconfiguration succeeded.

@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

Testing Ingress:

$ kubectl apply -f cafe.yaml

apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  name: coffee
spec:
  replicas: 2
  selector:
    matchLabels:
      app: coffee
  template:
    metadata:
      labels:
        app: coffee
    spec:
      containers:
      - name: coffee
        image: nginxdemos/hello:plain-text
        ports:
        - containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
  name: coffee-svc
spec:
  ports:
  - port: 80
    targetPort: 80
    protocol: TCP
    name: http
  selector:
    app: coffee
---
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  name: tea
spec:
  replicas: 3
  selector:
    matchLabels:
      app: tea
  template:
    metadata:
      labels:
        app: tea
    spec:
      containers:
      - name: tea
        image: nginxdemos/hello:plain-text
        ports:
        - containerPort: 80
---
apiVersion: v1
kind: Service
metadata:
  name: tea-svc
  labels:
spec:
  ports:
  - port: 80
    targetPort: 80
    protocol: TCP
    name: http
  selector:
    app: tea

$ kubectl apply -f ing.yaml

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
  name: cafe-ingress
  namespace: default
  selfLink: /apis/extensions/v1beta1/namespaces/default/ingresses/cafe-ingress
spec:
  rules:
  - http:
      paths:
      - backend:
          serviceName: tea-svc
          servicePort: 80
        path: /tea
      - backend:
          serviceName: coffee-svc
          servicePort: 80
        path: /coffee

$ minikube ip

192.168.99.100

$ curl 192.168.99.100/

default backend - 404

$ curl -L -k 192.168.99.100/tea

Server address: 172.17.0.11:80
Server name: tea-58d4697745-q9c8b
Date: 29/Aug/2018:20:22:58 +0000
URI: /tea
Request ID: 882ed5b066b6eff2c43f52fffb95f3c8

$ curl -L -k 192.168.99.100/coffee

Server address: 172.17.0.10:80
Server name: coffee-6c47b9cb9c-8tlkf
Date: 29/Aug/2018:20:22:46 +0000
URI: /coffee
Request ID: 22cfe7f478e0032b7d82bd06872e74b0

@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 29, 2018
@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

/assign @balopat

@diazjf
Copy link
Contributor Author

diazjf commented Aug 29, 2018

/assign @aledbf

@diazjf
Copy link
Contributor Author

diazjf commented Aug 30, 2018

Actually let's wait for 0.19. kubernetes/ingress-nginx#3014

@diazjf
Copy link
Contributor Author

diazjf commented Aug 30, 2018

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2018
@diazjf
Copy link
Contributor Author

diazjf commented Aug 30, 2018

$ kubectl logs -n kube-system nginx-ingress-controller-8566746984-fm2q5

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:    0.19.0
  Build:      git-05025d6
  Repository: https://github.com/kubernetes/ingress-nginx.git
-------------------------------------------------------------------------------

@diazjf
Copy link
Contributor Author

diazjf commented Aug 30, 2018

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 30, 2018
@diazjf
Copy link
Contributor Author

diazjf commented Aug 30, 2018

Ran the above tests on the new image as well. It's a good 💯

@dlorenc
Copy link
Contributor

dlorenc commented Sep 3, 2018

@minikube-bot OK to test

@dlorenc
Copy link
Contributor

dlorenc commented Sep 4, 2018

Looks like an issue with the ingress test:

    --- FAIL: TestFunctional/IngressController (604.43s)
    	addons_test.go:90: waiting for ingress-controller to be up: waiting for ingress-controller pods: timed out waiting for the condition

Are you able to repro this locally?

@diazjf
Copy link
Contributor Author

diazjf commented Sep 5, 2018

@dlorenc I was able to reproduce.

--- FAIL: TestFunctional/IngressController (605.69s)
        addons_test.go:90: waiting for ingress-controller to be up: waiting for ingress-controller pods: timed out waiting for the condition

I will need to look further into what is causing this. I think it's because I changed one of the labels!!

@@ -47,7 +47,7 @@ func testClusterStatus(t *testing.T) {
}
if status != api.ConditionTrue {
err := fmt.Errorf("Component %s is not Healthy! Status: %s", i.GetName(), status)
t.Log("Retrying, %s", err)
t.Logf("Retrying, %s", err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Formatting corrected. Was giving Error when running make integration:

➜  minikube git:(update-ingress-addon) make integration
go test -v -test.timeout=30m k8s.io/minikube/test/integration --tags="integration container_image_ostree_stub containers_image_openpgp" 
# k8s.io/minikube/test/integration
test/integration/cluster_status_test.go:50: Log call has possible formatting directive %s
FAIL    k8s.io/minikube/test/integration [build failed]
make: *** [integration] Error 2

@diazjf
Copy link
Contributor Author

diazjf commented Sep 5, 2018

@dlorenc resolved
=== RUN TestFunctional/IngressController
=== PAUSE TestFunctional/IngressController
=== CONT TestFunctional/IngressController
--- PASS: TestFunctional/IngressController (141.43s)

@diazjf
Copy link
Contributor Author

diazjf commented Sep 5, 2018

Not sure why the below is failing.

--- FAIL: TestFunctional/DNS (36.92s)
    	util.go:185: Error ;; connection timed out; no servers could be reached
    		
    		 running command [exec busybox-926xp nslookup kubernetes]. Return code: exit status 1
    	util.go:185: Error ;; connection timed out; no servers could be reached
    		
    		 running command [exec busybox-926xp nslookup kubernetes]. Return code: exit status 1
    	util.go:185: Error ;; connection timed out; no servers could be reached
    		
    		 running command [exec busybox-926xp nslookup kubernetes]. Return code: exit status 1
    	cluster_dns_test.go:62: running nslookup in pod:Temporary Error: Error running command. Error  exit status 1. Output: ;; connection timed out; no servers could be reached
    		
    		
    		Temporary Error: Error running command. Error  exit status 1. Output: ;; connection timed out; no servers could be reached
    		
    		
    		Temporary Error: Error running command. Error  exit status 1. Output: ;; connection timed out; no servers could be reached

@diazjf diazjf force-pushed the update-ingress-addon branch 2 times, most recently from 5b254df to cc497ee Compare September 6, 2018 15:18
@diazjf diazjf force-pushed the update-ingress-addon branch 2 times, most recently from 566ef93 to 34034f5 Compare September 17, 2018 00:37
@diazjf
Copy link
Contributor Author

diazjf commented Sep 17, 2018

@dlorenc different tests seem to fail and pass intermittently. Is this a known issue?

@diazjf diazjf force-pushed the update-ingress-addon branch 2 times, most recently from 9e2252b to 09ad59f Compare September 26, 2018 18:34
@tstromberg
Copy link
Contributor

@dlorenc different tests seem to fail and pass intermittently. Is this a known issue?

It's a problem on our end: #3163

- Updates Ingress-Controller Version to 0.19.0
- Adds Service Account for Ingress-Controller
- Adds Support for Prometheus
- Fixes bug with TCP/UDP ConfigMaps not Loading
- Adds more resource limits to default-backend
- Use new ingress class name
- Use app.kubernetes.io/xxxxxxxxxxx labels
@tstromberg tstromberg added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 1, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: diazjf

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@diazjf
Copy link
Contributor Author

diazjf commented Oct 4, 2018

@tstromberg thanks for approving! How does the merge process work after approval?

@balopat balopat merged commit 2ebdf5e into kubernetes:master Oct 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants