Remove deprecated 'enable-cri' flag in kubernetes 1.7 #1408

amitkumarj441 · 2017-07-02T22:37:33Z

Signed-off-by: Amit Kumar Jaiswal amitkumarj441@gmail.com

Signed-off-by: Amit Kumar Jaiswal <amitkumarj441@gmail.com>

k8s-ci-robot · 2017-07-02T22:37:35Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

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. I understand the commands that are listed here.

matlockx · 2017-07-03T09:19:13Z

roles/kubernetes/node/templates/kubelet.j2

@@ -13,7 +13,7 @@ KUBELET_HOSTNAME="--hostname-override={{ ansible_hostname }}"
 --pod-infra-container-image={{ pod_infra_image_repo }}:{{ pod_infra_image_tag }} \
 --kube-reserved cpu={{ kubelet_cpu_limit }},memory={{ kubelet_memory_limit|regex_replace('Mi', 'M') }} \
 --node-status-update-frequency={{ kubelet_status_update_frequency }} \
-{% if kube_version | version_compare('v1.6', '>=') %}
+{% if kube_version | version_compare('v1.7', '>=') %}
 --enable-cri={{ kubelet_enable_cri }} --cgroups-per-qos={{ kubelet_cgroups_per_qos }} \


this won't work. enable-cri was removed in 1.7.0

@matlockx But as I tested, adding a kube_version check for --enable-cri was enough to upgrade to 1.7.0 using upgrade-cluster (with default options).

interesting. i just tested it and kubelet service failed to start with 'Error: unknown flag: --enable-cri'. maybe you haven't changed the version to 1.7.0? because the check just excludes the block from being used if your version is still 1.6.* and on top of that it does not make sense to me to change the check like that. i think it should be something like

{% if kube_version | version_compare('v1.6', '>=') %} {# flag got removed with 1.7.0 #} {% if kube_version | version_compare('v1.7', '<') %} --enable-cri={{ kubelet_enable_cri }} {% endif %}

what does kubectl version say on your cluster.

see also https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG.md#server-binaries

@matlockx Alright. On my cluster kubectl version is 1.5.6 and you're right with your suggestion.

Thanks for reminding me 😀 !

Can I update the changes you suggested above?

sure. but i'm not a maintainer, so it has to go through review by one of them anyhow.

Okay @matlockx ! Just updated it.

Updated!!

Updated!

matlockx · 2017-07-03T10:38:13Z

roles/kubernetes/node/templates/kubelet.j2

@@ -14,6 +14,8 @@ KUBELET_HOSTNAME="--hostname-override={{ ansible_hostname }}"
 --kube-reserved cpu={{ kubelet_cpu_limit }},memory={{ kubelet_memory_limit|regex_replace('Mi', 'M') }} \
 --node-status-update-frequency={{ kubelet_status_update_frequency }} \
 {% if kube_version | version_compare('v1.6', '>=') %}
+{# flag got removed with 1.7.0 #}
+{% if kube_version | version_compare('v1.7', '<') %}
 --enable-cri={{ kubelet_enable_cri }} --cgroups-per-qos={{ kubelet_cgroups_per_qos }} \


you forgot the closing endif, it must be:

{% if kube_version | version_compare('v1.6', '>=') %} {# flag got removed with 1.7.0 #} {% if kube_version | version_compare('v1.7', '<') %} --enable-cri={{ kubelet_enable_cri }} \ {% endif %} --cgroups-per-qos={{ kubelet_cgroups_per_qos }} \

anyhow, you should double check the file after the jinja2 replacements. i haven't checked it.

Thanks @matlockx for pointing this out.

you are welcome

Updated with closing endif.

matlockx · 2017-07-03T11:10:45Z

ci check this

rsmitty · 2017-07-06T12:58:25Z

ci check this

matlockx · 2017-07-06T13:20:41Z

@rsmitty i see, you got the ci super power :)

matlockx · 2017-07-10T10:10:07Z

ok, how do you read the ci result? https://gitlab.com/kargo-ci/kubernetes-incubator__kubespray/pipelines/9534059 says 'passed' but it still says here 'failed' and: any other suggestions regarding the change?

bradbeam · 2017-07-10T13:05:04Z

It's just the webhooks being dumb, CI passed

matlockx · 2017-07-11T08:25:03Z

can we merge this?

matlockx · 2017-07-11T14:07:47Z

awesome! thanks

Updated K8s version

46d3f43

Signed-off-by: Amit Kumar Jaiswal <amitkumarj441@gmail.com>

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 2, 2017

matlockx reviewed Jul 3, 2017

View reviewed changes

Update kubelet.j2

c36fb59

Updated!!

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 3, 2017

Update kubelet.j2

3d2680a

Updated!

matlockx reviewed Jul 3, 2017

View reviewed changes

Update kubelet.j2

319a0d6

Updated with closing endif.

hzamani mentioned this pull request Jul 5, 2017

Kubernetes 1.7 support #1404

Closed

bradbeam changed the title ~~Updated Kubernetes version~~ Remove deprecated 'enable-cri' flag in kubernetes 1.7 Jul 10, 2017

bradbeam approved these changes Jul 10, 2017

View reviewed changes

matlockx approved these changes Jul 11, 2017

View reviewed changes

rsmitty merged commit c75b21a into kubernetes-sigs:master Jul 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove deprecated 'enable-cri' flag in kubernetes 1.7 #1408

Remove deprecated 'enable-cri' flag in kubernetes 1.7 #1408

amitkumarj441 commented Jul 2, 2017

k8s-ci-robot commented Jul 2, 2017

matlockx Jul 3, 2017

amitkumarj441 Jul 3, 2017

matlockx Jul 3, 2017

matlockx Jul 3, 2017

amitkumarj441 Jul 3, 2017 •

edited

Loading

matlockx Jul 3, 2017

amitkumarj441 Jul 3, 2017

matlockx Jul 3, 2017

matlockx Jul 3, 2017

amitkumarj441 Jul 3, 2017

matlockx Jul 3, 2017

matlockx commented Jul 3, 2017

rsmitty commented Jul 6, 2017

matlockx commented Jul 6, 2017

matlockx commented Jul 10, 2017

bradbeam commented Jul 10, 2017

matlockx commented Jul 11, 2017

matlockx commented Jul 11, 2017 •

edited

Loading

Remove deprecated 'enable-cri' flag in kubernetes 1.7 #1408

Remove deprecated 'enable-cri' flag in kubernetes 1.7 #1408

Conversation

amitkumarj441 commented Jul 2, 2017

k8s-ci-robot commented Jul 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amitkumarj441 Jul 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matlockx commented Jul 3, 2017

rsmitty commented Jul 6, 2017

matlockx commented Jul 6, 2017

matlockx commented Jul 10, 2017

bradbeam commented Jul 10, 2017

matlockx commented Jul 11, 2017

matlockx commented Jul 11, 2017 • edited Loading

amitkumarj441 Jul 3, 2017 •

edited

Loading

matlockx commented Jul 11, 2017 •

edited

Loading