Feature/k8s win nodes hybrid cluster #2956

pablodav · 2018-07-03T13:00:43Z

refs #2889

Add nodeSelector for some deployments and pods to have compatibility in hybrid clusters.
fix for missing default value in item.container for downloads
fix for missing Pin priority for debian based

nodeSelector, not added to all deployments yet, it could be evaluated later. (But could be required in future)

…ocker to incompatible version

… into feature/k8s_win

…ocker to incompatible version

… into feature/k8s_win

…ocker to incompatible version

… into feature/k8s_win

… nodes, also fix for download with missing container type

ant31 · 2018-07-04T09:35:48Z

ci check this

ant31 · 2018-07-04T09:38:03Z

roles/dnsmasq/templates/dnsmasq-autoscaler.yml.j2

@@ -54,3 +54,8 @@ spec:
            - --default-params={"linear":{"nodesPerReplica":{{ dnsmasq_nodes_per_replica }},"preventSinglePointFailure":true}}
            - --logtostderr=true
            - --v={{ kube_log_level }}
+{% if kube_patch_win_nodes %}
+      # When having win nodes in cluster without this patch, this pod cloud try to be created in windows
+      nodeSelector:


isn't safe to always add this selector ?

In the past I have sent merge request with the selector added without if and someone else asked to add something like this.

Here I found @woopstar disliked the idea to have the nodeselector: #2556

where did you find this label: beta.kubernetes.io/os is it documented somewhere, is it used by other installer (kops, kubeadm ?)

I think it's perfectly legit to add it, https://kubernetes.io/docs/reference/kubernetes-api/labels-annotations-taints/#beta-kubernetes-io-os

ant31 · 2018-07-04T09:38:29Z

roles/kubernetes/master/tasks/kubeadm-setup.yml

@@ -89,6 +89,7 @@
    --ignore-preflight-errors=all
    --allow-experimental-upgrades
    --allow-release-candidate-upgrades
+    --force


Looks it got inserted but I forgot to remove, anyway it is safe to have this line.
In past two tries to upgrade kubeadm, it failed without --force and asked to add --force flag (tried from v1.10.3 to v1.10.4 and from v1.10.4 to v1.10.5

ant31 · 2018-07-04T09:40:41Z

Why is this specific to windows, can't we simply add the selector for all kind of installation (remove the 'if' and add the selector in all manifests

Atoms · 2018-07-04T10:47:10Z

agree with @ant31 on selectors

also my comment would be about syntax:

tags:
  - tag1
  - tag2

not

tags: [tag1,tag2]

and same goes for when statements

when:
  - something
  - something else

this way it's more readable

and please squash your commits.

pablodav · 2018-07-04T13:08:21Z

Here I found @woopstar disliked the idea to have the nodeselector: #2556

@Atoms,
I like the syntax you proposed, but I have just copied the syntax that other tasks already have in this role.

Atoms · 2018-07-04T13:10:04Z

@pablodav this syntax change was introduced when i joined project and not all places changed this so it would be grate if you could change.

pablodav · 2018-07-04T13:28:14Z

cluster.yml

@@ -93,6 +93,7 @@
  roles:
    - { role: kubespray-defaults}
    - { role: kubernetes-apps/rotate_tokens, tags: rotate_tokens, when: "secret_changed|default(false)" }
+    - { role: win_nodes/kubernetes_patch, tags: win_nodes, when: "kubeadm_enabled and kube_patch_win_nodes" }


Thanks @Atoms

for when, only this needs to be changed?

Could be?

roles: - role: kubespray-defaults - role: kubernetes-apps/rotate_tokens tags: - rotate_tokens when: - secret_changed|default(false) - role: win_nodes/kubernetes_patch tags: - win_nodes when: - kubeadm_enabled - and kube_patch_win_nodes

I haven't found examples to understand this syntax better

not in playbook file, but in tasks files, i think for roles still not working such syntax (need to check)

Ok, haven't found other line I have changed to change now, probably could be reviewed in future PRs.

ant31 · 2018-07-04T15:01:50Z

I'm in favor to do the nodeSelector to all deployments.
It doesn't break anything to add this nodeSelector, it adds more precision/safety.
As we move forward into multi-os support (linux/windows) it's going to be required. And I largely prefer not to if guard every piece of code.
cc @mattymo ?

CheckNodePIDPressure is not supported in v1.10

Add TF_VAR_ to terraform env

* Add tags for cert serial tasks This will help facilitate tag-based deployment of specific components. * fixup kubernetes node

…ocker to incompatible version

… nodes, also fix for download with missing container type

… into feature/k8s_win

Atoms · 2018-07-06T17:12:26Z

@pablodav you did something wrong :) you don't need to add commits from another people :)

pablodav · 2018-07-09T02:54:33Z

You are right, looks like when I sync upstream using rebase it adds commits and the pull request is synced with those changes incorrectly, will create new clean pull request.

pablodav · 2018-07-09T02:58:25Z

Created new in #2978, please @Atoms, @ant31 check that new merge request.
I'm will try to not add more synchronizations from upstream to my branch for a while.

pablodav and others added 14 commits June 5, 2018 08:15

ensure there is pin priority for docker package to avoid upgrade of d…

a085569

…ocker to incompatible version

ensure there is pin priority for docker package to avoid upgrade of d…

1185836

…ocker to incompatible version

Merge branch 'feature/k8s_win' of https://github.com/pablodav/kubespray…

7a08ede

… into feature/k8s_win

remove empty when line

51e9efa

ensure there is pin priority for docker package to avoid upgrade of d…

39165be

…ocker to incompatible version

remove empty when line

62e80bc

force kubeadm upgrade due to failure without --force flag

5cb8761

Merge branch 'feature/k8s_win' of https://github.com/pablodav/kubespray…

34bc7a2

… into feature/k8s_win

CheckNodePIDPressure is not supported in v1.10

2a279e3

ensure there is pin priority for docker package to avoid upgrade of d…

3b773e6

…ocker to incompatible version

remove empty when line

4e40d9a

force kubeadm upgrade due to failure without --force flag

6c069bc

Merge branch 'feature/k8s_win' of https://github.com/pablodav/kubespray…

e62eb1d

… into feature/k8s_win

added nodeSelector to have compatibility with hybrid cluster with win…

89d4424

… nodes, also fix for download with missing container type

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 3, 2018

pablodav mentioned this pull request Jul 3, 2018

Support windows #2889

Closed

ant31 requested a review from mattymo July 4, 2018 09:35

ant31 reviewed Jul 4, 2018

View reviewed changes

Atoms added the feature label Jul 4, 2018

fixes in syntax and LF for newline in files

e6f8259

fix on yamllint check

67e15c4

pablodav commented Jul 4, 2018

View reviewed changes

pablodav mentioned this pull request Jul 4, 2018

More devs working on this project pablodav/kubernetes-for-windows#8

Closed

Atoms and others added 11 commits July 4, 2018 18:30

Merge pull request #2946 from Miouge1/remove-pid-predicate

4092f96

CheckNodePIDPressure is not supported in v1.10

Fix terraform env Not effective (#2966)

4d7426e

Add TF_VAR_ to terraform env

Improve vault etcd initialization check (#2959)

0b939a4

Add tags to deploy components by --tags option (#2960)

5c617c5

* Add tags for cert serial tasks This will help facilitate tag-based deployment of specific components. * fixup kubernetes node

ensure there is pin priority for docker package to avoid upgrade of d…

915ea26

…ocker to incompatible version

remove empty when line

0d1bd1e

force kubeadm upgrade due to failure without --force flag

f32a14b

added nodeSelector to have compatibility with hybrid cluster with win…

d961d40

… nodes, also fix for download with missing container type

fixes in syntax and LF for newline in files

d1af8bd

fix on yamllint check

3fad479

Merge branch 'feature/k8s_win' of https://github.com/pablodav/kubespray…

a8a4265

… into feature/k8s_win

pablodav closed this Jul 9, 2018

pablodav mentioned this pull request Jul 9, 2018

Feature/k8s win #2978

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/k8s win nodes hybrid cluster #2956

Feature/k8s win nodes hybrid cluster #2956

pablodav commented Jul 3, 2018

ant31 commented Jul 4, 2018

ant31 Jul 4, 2018

pablodav Jul 4, 2018

pablodav Jul 4, 2018 •

edited

Loading

ant31 Jul 4, 2018

ant31 Jul 4, 2018 •

edited

Loading

ant31 Jul 4, 2018

pablodav Jul 4, 2018

ant31 commented Jul 4, 2018

Atoms commented Jul 4, 2018

pablodav commented Jul 4, 2018

Atoms commented Jul 4, 2018

pablodav Jul 4, 2018

Atoms Jul 4, 2018

pablodav Jul 4, 2018

ant31 commented Jul 4, 2018

Atoms commented Jul 6, 2018

pablodav commented Jul 9, 2018

pablodav commented Jul 9, 2018

Feature/k8s win nodes hybrid cluster #2956

Feature/k8s win nodes hybrid cluster #2956

Conversation

pablodav commented Jul 3, 2018

ant31 commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pablodav Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ant31 Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ant31 commented Jul 4, 2018

Atoms commented Jul 4, 2018

pablodav commented Jul 4, 2018

Atoms commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ant31 commented Jul 4, 2018

Atoms commented Jul 6, 2018

pablodav commented Jul 9, 2018

pablodav commented Jul 9, 2018

pablodav Jul 4, 2018 •

edited

Loading

ant31 Jul 4, 2018 •

edited

Loading