kube_proxy_and_dns: add role that runs standalone kube-proxy and DNS by dcbw · Pull Request #9621 · openshift/openshift-ansible

dcbw · 2018-08-16T03:13:57Z

And depend on that role in Calico.

@smarterclayton @sdodson @knobunc @squeed 3rd party plugins might depend on kube-proxy, which as of 3.10 is part of the SDN daemonset which those 3rd party plugins probably don't want to select/run. Including an ansible role for it is one option; another would be to request that each 3rd party plugin that needs kube-proxy add a container for it to their pod definition in their daemonset. Thoughts?

(also my Ansible is weak, I have no idea if this is the correct way to do it)

vrutkovs · 2018-08-16T09:49:52Z

This PR (and fixes from #9435) seem to fix Calico on 3.11. Could you resubmit it for master as well?

dcbw · 2018-08-16T18:28:19Z

Could you resubmit it for master as well?

@vrutkovs Yes, I can do that as well. The immediate issue we had was lack of kube-proxy in 3.10 due to the daemonset requirements, hence the 3.10 submission as the first pass. I'd like to get some comments on it too, are you able to test this PR out and verify that it correctly starts the kube-proxy daemonset on calico nodes?

vrutkovs · 2018-08-16T21:09:54Z

@dcbw yep, kube-proxy daemonset started and calico worked fine (with several fixes from #9435 - would be great if you could have a look at those if possible). Haven't tried other custom SDNs.

Ansible part looks great, lets have another lgtm about the approach and we'll merge this.

I wonder if running kube-proxy in a daemonset would change the metrics fix in #9592?

spuranam

Its best not to hardcode version instead something like this should do the trick s/v3.10/${version}/

spuranam · 2018-08-17T03:13:07Z

roles/kube_proxy/files/kube-proxy-images.yaml

+apiVersion: image.openshift.io/v1
+kind: ImageStreamTag
+metadata:
+  name: node:v3.10


Its best not to hardcode version instead something like this should do the trick s/v3.10/${version}/

This is a name of the imagestream, it needs to be hardcoded so that an upgraded cluster would have several imagestreams for major versions

Yeah, for a PR against 'git master' I would expect the tag to be different, but I was just copying what some of the existing ImageStream tags already were thus "v3.10"

spuranam · 2018-08-17T03:13:25Z

roles/kube_proxy/files/kube-proxy-images.yaml

+  reference: true
+  from:
+    kind: DockerImage
+    name: openshift/node:v3.10.0


Its best not to hardcode version instead something like this should do the trick s/v3.10/${version}/

It would be updated in https://github.com/openshift/openshift-ansible/pull/9621/files#diff-eaf9b159b10b04891c4d8f55d2352accR21

spuranam · 2018-08-17T03:13:43Z

roles/kube_proxy/files/kube-proxy.yaml

+      This daemonset launches kube-proxy.
+    image.openshift.io/triggers: |
+      [
+        {"from":{"kind":"ImageStreamTag","name":"node:v3.10"},"fieldPath":"spec.template.spec.containers[?(@.name==\"proxy\")].image"}


Its best not to hardcode version instead something like this should do the trick s/v3.10/${version}/

squeed · 2018-08-17T12:25:29Z

Should this use the openshift binary or the hyperkube binary? My preference would be for hyperkube.

smarterclayton · 2018-08-17T20:01:20Z

Standalone is probably better, since this is a workaround until we ship 4.0

…

On Fri, Aug 17, 2018 at 8:25 AM Casey Callendrello ***@***.***> wrote: Should this use the openshift binary or the hyperkube binary? My preference would be for hyperkube. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9621 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p5IKRaku7gt_zNVP4CEBiYECYWMeks5uRrZAgaJpZM4V_IZN> .

kprabhak · 2018-08-17T12:46:17Z

roles/kube_proxy/files/kube-proxy.yaml

+          oc config --config=/tmp/kubeconfig set-credentials sa "--token=$( cat /var/run/secrets/kubernetes.io/serviceaccount/token )"
+          oc config --config=/tmp/kubeconfig set-context "$( oc config --config=/tmp/kubeconfig current-context )" --user=sa
+          # Launch the kube-proxy process
+          exec openshift start network --disable=dns,plugins --enable=proxy --config=/etc/origin/node/node-config.yaml --kubeconfig=/tmp/kubeconfig --loglevel=${DEBUG_LOGLEVEL:-2}


You might want to "--enable=dns", otherwise cluster internal service lookups may not work.

@dcbw @vrutkovs @kprabhak I can confirm that this PR along with @kprabhak suggestion and fixes from #9435 fixes Calico on OpenShit Enterprise 3.10

@kprabhak yes good point, which brings up the questions does everyone who would use a standalone kube-proxy role also use the DNS, and if not, should it be individually selectable from kube-proxy.

For what it is worth, I had:

exec openshift start network --disable=plugins --enable=proxy,dns --config=/etc/origin/node/node-config.yaml --kubeconfig=/tmp/kubeconfig --loglevel=${DEBUG_LOGLEVEL:-2}

Just work for me in a install.

mgleung · 2018-08-19T19:13:26Z

roles/calico/meta/main.yml

 dependencies:
 - role: openshift_facts
 - role: container_runtime
+- role: kube_proxy


In the release-3.10 branch, this role dependency should go in the calico_master role. If this is still a thing going forward, we should leave this in the calico role (we will rename the roles going forward as per #9435). I made a PR for the appropriate calico fixes in the release-3.10 branch here: #9657 but this means that this role will need to be a dependency of the calico_master role instead of the calico role.

exactly what @mgleung , for the 3.10 release branch this should be under calico_master role.

And depend on that role in Calico.

dcbw · 2018-08-30T20:42:24Z

@mgleung @dmmcquay @vrutkovs @kprabhak @spuranam

Updated the role depenency to calico_master and added DNS to the service. Thus the updated name. PTAL thanks!

dcbw · 2018-08-31T01:00:04Z

/retest

kprabhak · 2018-08-31T18:39:47Z

Looks like one pr previously merged into 3.9 but left behind in the new kube-proxy structure in 3.10 is #8099

Without this, nodePorts only work in cases where the endpoint happens to be on the same node.

In 3.10, the location for roles/openshift_node/templates/node.yaml.v1.j2 appears to have changed to roles/openshift_node_group/templates/node-config.yaml.j2

I think @mgleung is working on a separate PR for this - I can confirm that nodePorts work correctly from all nodes once the following lines are added to roles/openshift_node_group/templates/node-config.yaml.j2:
{% if openshift_use_calico | default(False) | bool %}
proxyArguments:
cluster-cidr:
- {{ osm_cluster_network_cidr }}
{% endif %}

mgleung · 2018-08-31T19:19:33Z

I added #9863 and #9866 for the changes @kprabhak described.

The changes now look good to me. Thanks @dcbw !

sdodson · 2018-08-31T20:35:55Z

Ansible code looks fine to me. Just need to make sure to forward port this.
/lgtm

openshift-ci-robot · 2018-08-31T20:36:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, sdodson

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [sdodson]

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

kube_proxy_and_dns: add role that runs standalone kube-proxy and DNS

mgleung · 2018-08-31T23:09:41Z

I created a forward port of this with the changes from #9871 added on as well here: #9872

vrutkovs · 2018-09-03T10:53:14Z

/cherrypick master

openshift-cherrypick-robot · 2018-09-03T10:53:22Z

@vrutkovs: new pull request created: #9878

Details

In response to this:

/cherrypick master

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 16, 2018

openshift-ci-robot requested review from sdodson and vrutkovs August 16, 2018 03:14

dcbw force-pushed the 3.10-standalone-kube-proxy branch from dc7c79e to 0fe2381 Compare August 16, 2018 03:18

spuranam reviewed Aug 17, 2018

View reviewed changes

mgleung mentioned this pull request Aug 17, 2018

Update playbooks for Calico in OpenShift 3.10 #9657

Merged

kprabhak reviewed Aug 18, 2018

View reviewed changes

mgleung reviewed Aug 19, 2018

View reviewed changes

kube_proxy_and_dns: add role that runs standalone kube-proxy + DNS

67e911f

And depend on that role in Calico.

dcbw force-pushed the 3.10-standalone-kube-proxy branch from 0fe2381 to 67e911f Compare August 30, 2018 20:40

dcbw changed the title ~~[RFC] kube_proxy: add role that runs standalone kube-proxy~~ kube_proxy_and_dns: add role that runs standalone kube-proxy and DNS Aug 31, 2018

openshift-ci-robot assigned sdodson Aug 31, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 31, 2018

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2018

openshift-merge-robot merged commit eeb4272 into openshift:release-3.10 Aug 31, 2018

mgleung pushed a commit to mgleung/openshift-ansible that referenced this pull request Aug 31, 2018

Merge pull request openshift#9621 from dcbw/3.10-standalone-kube-proxy

7debaaf

kube_proxy_and_dns: add role that runs standalone kube-proxy and DNS

mgleung mentioned this pull request Aug 31, 2018

Forward port kube proxy role and run it only once for Calico installs #9872

Closed

openshift-cherrypick-robot mentioned this pull request Sep 3, 2018

[master] kube_proxy_and_dns: add role that runs standalone kube-proxy and DNS #9878

Merged

Conversation

dcbw commented Aug 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vrutkovs commented Aug 16, 2018

Uh oh!

dcbw commented Aug 16, 2018

Uh oh!

vrutkovs commented Aug 16, 2018

Uh oh!

spuranam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

squeed commented Aug 17, 2018

Uh oh!

smarterclayton commented Aug 17, 2018 via email

Uh oh!

kprabhak Aug 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgleung Aug 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcbw commented Aug 30, 2018

Uh oh!

dcbw commented Aug 31, 2018

Uh oh!

kprabhak commented Aug 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgleung commented Aug 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdodson commented Aug 31, 2018

Uh oh!

openshift-ci-robot commented Aug 31, 2018

Uh oh!

mgleung commented Aug 31, 2018

Uh oh!

vrutkovs commented Sep 3, 2018

Uh oh!

openshift-cherrypick-robot commented Sep 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

dcbw commented Aug 16, 2018 •

edited

Loading

kprabhak Aug 17, 2018 •

edited

Loading

mgleung Aug 19, 2018 •

edited

Loading

kprabhak commented Aug 31, 2018 •

edited

Loading

mgleung commented Aug 31, 2018 •

edited

Loading