Skip to content

Conversation

nueavv
Copy link
Contributor

@nueavv nueavv commented Aug 9, 2025

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #41043

@nueavv nueavv requested a review from a team as a code owner August 9, 2025 08:47
@maintainer-s-little-helper
Copy link

Commit f37d8a7 does not match "(?m)^Signed-off-by:".

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

@nueavv nueavv requested a review from qmonnet August 9, 2025 08:47
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Aug 9, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Aug 9, 2025
@aanm aanm requested a review from dylandreimerink August 11, 2025 08:02
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thank you!

Could you please add a bit more context to your commit? Could you have a link to the commit or version notes that removed it in your commit description, for example, please?

We also require that you sign off your commit, as explained in the comment above, please.

@qmonnet qmonnet changed the title Documentation: remove stale mention of externalIPs.enabled docs: Remove stale mention of externalIPs.enabled Aug 11, 2025
@qmonnet qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/cleanup This includes no functional changes. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Aug 11, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Aug 11, 2025
@nueavv
Copy link
Contributor Author

nueavv commented Aug 11, 2025

@qmonnet Thanks for the review! I’ll update the commit shortly to add more context and a link to the Upgrade Guide. and this commit

also I’ll add a DCO sign-off.

The docs change will align with the fact that in 1.17.x the Helm chart has already removed these flags and that kube-proxy replacement features are gated by --kube-proxy-replacement=true.

@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 Aug 12, 2025
@nueavv nueavv requested a review from qmonnet August 12, 2025 12:54
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

This is very complete now, thank you!

@qmonnet
Copy link
Member

qmonnet commented Aug 12, 2025

/test

@qmonnet qmonnet added release-note/misc This PR makes changes that have no direct user impact. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Aug 12, 2025
The docs still reference Helm flags that are no longer used in recent
Cilium Helm charts. Since 1.17.6, these options were removed from the chart,
and the kube-proxy replacement features are controlled via
--kube-proxy-replacement=true instead.
cilium@6a51743

Per the upgrade guide:
"The flags --enable-node-port (nodePort.enabled in Helm), --enable-host-port,
--enable-external-ips have been deprecated and will be removed in Cilium 1.19.
The kube-proxy replacement features will be only enabled when
--kube-proxy-replacement is set to true."
https://docs.cilium.io/en/stable/operations/upgrade/

This commit updates the docs to remove the stale Helm flags and align the
instructions with current behavior.

Signed-off-by: nueavv <nuguni@kakao.com>
@nueavv
Copy link
Contributor Author

nueavv commented Aug 13, 2025

/test

@nueavv
Copy link
Contributor Author

nueavv commented Aug 13, 2025

@qmonnet, thanks again for reviewing my PR!
I tried merging it, but I got this message: "You're not authorized to push to this branch."
Do you know if I need special permissions, or should someone else with access merge it?

@qmonnet
Copy link
Member

qmonnet commented Aug 13, 2025

/test

@qmonnet
Copy link
Member

qmonnet commented Aug 13, 2025

Do you know if I need special permissions

Correct, only Cilium Committers have write access and can merge to the repository.

I was waiting for Dylan to review as well, but from his GitHub status I guess he's on holiday. Let's merge this 🙂

@qmonnet qmonnet added this pull request to the merge queue Aug 13, 2025
Merged via the queue into cilium:main with commit cb2cfc8 Aug 13, 2025
65 checks passed
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. kind/cleanup This includes no functional changes. kind/community-contribution This was a contribution made by a community member. 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.

Docs: L2 Announcements still reference externalIPs.enabled=true Helm option (removed)
2 participants