Skip to content

Set 'kubernetes' IPAM in kube-proxy free guide#12246

Merged
aanm merged 1 commit intocilium:masterfrom
frankcui95:helm-pod-cidr-to-10-217-0-0
Jul 1, 2020
Merged

Set 'kubernetes' IPAM in kube-proxy free guide#12246
aanm merged 1 commit intocilium:masterfrom
frankcui95:helm-pod-cidr-to-10-217-0-0

Conversation

@frankcui95
Copy link
Copy Markdown

The pod cidr should match 10.217.0.0/16 mentioned previously,
not the default value 10.0.0.0/16 set by default via helm in this tutorial.
Appended '--set global.ipam.operator.clusterPoolIPv4PodCIDR=10.217.0.0/16' to the end of the helm command line.

Tested with
Kubernetes v1.17.5
cri-o v1.17

I know none about ipam, and may be blinded

Is this difference ( not match cidr assigned by kube-controller-manager) OK?

@frankcui95 frankcui95 requested a review from a team as a code owner June 23, 2020 21:56
@maintainer-s-little-helper
Copy link
Copy Markdown

Commit a73e68781a908ab761c4ff9664abcfcbbfa8e564 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper Bot added backport/1.8 kind/backports This PR provides functionality previously merged into master. dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. labels Jun 23, 2020
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR, this should be --set config.ipam=kubernetes instead of --set global.ipam.operator.clusterPoolIPv4PodCIDR=10.217.0.0/16

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@aanm Thank you for tell me that。 i'll deploy again

@aanm
Copy link
Copy Markdown
Member

aanm commented Jun 24, 2020

@Maireo I also changed the base branch to be master because we need to fix this first in the master and after being merged we will perform the backport to 1.8.

Can you sing your commit as instructed in #12246 (comment)? Thanks

@aanm aanm changed the base branch from v1.8 to master June 24, 2020 10:08
@aanm aanm force-pushed the helm-pod-cidr-to-10-217-0-0 branch from a73e687 to 543a9b0 Compare June 24, 2020 10:10
@frankcui95 frankcui95 requested a review from a team as a code owner June 24, 2020 10:10
@maintainer-s-little-helper
Copy link
Copy Markdown

Commit 543a9b08d902625e15c67f18eacde29bd1e6ce7d does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@aanm aanm added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. needs-backport/1.8 and removed backport/1.8 dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. kind/backports This PR provides functionality previously merged into master. labels Jun 24, 2020
@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

5 similar comments
@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@aanm aanm added the release-note/misc This PR makes changes that have no direct user impact. label Jun 24, 2020
@aanm aanm changed the title The pod cidr should match 10.217.0.0/16 Set 'kubernetes' IPAM in kube-proxy free guide Jun 24, 2020
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 24, 2020

Coverage Status

Coverage decreased (-0.2%) to 36.928% when pulling 46004cac297af02e29c78632aef68964c6f63305 on MaiReo:helm-pod-cidr-to-10-217-0-0 into 55dd1b9 on cilium:master.

@maintainer-s-little-helper
Copy link
Copy Markdown

Commits 543a9b08d902625e15c67f18eacde29bd1e6ce7d, 5136d26feb5a2a82301f199d34043b60cfdcffb1 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper Bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 29, 2020
@maintainer-s-little-helper
Copy link
Copy Markdown

Commits 543a9b08d902625e15c67f18eacde29bd1e6ce7d, 5136d26feb5a2a82301f199d34043b60cfdcffb1 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@frankcui95
Copy link
Copy Markdown
Author

I forgot to set Signed-off-by when commiting. Should I to close the PR?

@aanm
Copy link
Copy Markdown
Member

aanm commented Jun 29, 2020

I forgot to set Signed-off-by when commiting. Should I to close the PR?

@Maireo no need for that, you can squash all your commits with git rebase -i origin/master and choose to squash the lasts 2 commits together with the first one.

The pod cidr should match 10.217.0.0/16 mentioned previously,
not the default value 10.0.0.0/16 set by default via helm in this tutorial.
--set global.ipam.operator.clusterPoolIPv4PodCIDR=10.217.0.0/16

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: MaiReo <sawako.saki@gmail.com>
@aanm aanm force-pushed the helm-pod-cidr-to-10-217-0-0 branch from 46004ca to 056b5ef Compare July 1, 2020 10:37
@maintainer-s-little-helper maintainer-s-little-helper Bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jul 1, 2020
@aanm aanm merged commit ea8b727 into cilium:master Jul 1, 2020
brb added a commit that referenced this pull request Jul 6, 2020
It's no longer required to specify podCIDR [1] when provisioning
k8s with kubeadm for Cilium. Removing this allows us to simplify
the guide by getting rid of passing the ipam mode to helm which
was introduced by #12246.

[1]: kubernetes/website#21432 (comment)

Signed-off-by: Martynas Pumputis <m@lambda.lt>
borkmann pushed a commit that referenced this pull request Jul 6, 2020
It's no longer required to specify podCIDR [1] when provisioning
k8s with kubeadm for Cilium. Removing this allows us to simplify
the guide by getting rid of passing the ipam mode to helm which
was introduced by #12246.

[1]: kubernetes/website#21432 (comment)

Signed-off-by: Martynas Pumputis <m@lambda.lt>
vadorovsky pushed a commit that referenced this pull request Jul 7, 2020
[ upstream commit d131945 ]

It's no longer required to specify podCIDR [1] when provisioning
k8s with kubeadm for Cilium. Removing this allows us to simplify
the guide by getting rid of passing the ipam mode to helm which
was introduced by #12246.

[1]: kubernetes/website#21432 (comment)

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
qmonnet pushed a commit that referenced this pull request Jul 15, 2020
[ upstream commit d131945 ]

It's no longer required to specify podCIDR [1] when provisioning
k8s with kubeadm for Cilium. Removing this allows us to simplify
the guide by getting rid of passing the ipam mode to helm which
was introduced by #12246.

[1]: kubernetes/website#21432 (comment)

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants