Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove deprecated v1.10 options #14291

Merged
merged 4 commits into from
Dec 8, 2020
Merged

Remove deprecated v1.10 options #14291

merged 4 commits into from
Dec 8, 2020

Conversation

jibi
Copy link
Member

@jibi jibi commented Dec 7, 2020 •
edited

Remove deprecated v1.10 options:

  • k8s-watcher-queue-size
  • blacklist-conflicting-routes
  • device
  • crd-wait-timeout (operator)

I've requested the review of André, Deepesh, Martynas and Chris as you have marked these options as deprecated as so should have the most context on them.

@jibi jibi requested review from brb, tklauser, aanm, fristonio and a team December 7, 2020 10:39
@jibi jibi requested review from a team as code owners December 7, 2020 10:39
@jibi jibi requested review from a team and qmonnet December 7, 2020 10:39
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 7, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Dec 7, 2020
@jibi jibi added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Dec 7, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 7, 2020
@jibi jibi added area/cli Impacts the command line interface of any command in the repository. area/daemon Impacts operation of the Cilium daemon. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. labels Dec 7, 2020
aanm
aanm requested changes Dec 7, 2020
View reviewed changes
Documentation/operations/upgrade.rst Outdated Show resolved Hide resolved
Documentation/operations/upgrade.rst Outdated Show resolved Hide resolved
daemon/cmd/daemon_main.go Outdated
Comment on lines 477 to 478
flags.String(option.K8sWatcherEndpointSelector, defaults.K8sWatcherEndpointSelector, "K8s endpoint watcher will watch for these k8s endpoints")
option.BindEnv(option.K8sWatcherEndpointSelector)
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be removed

Copy link
Member Author

Choose a reason for hiding this comment

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

ENOCOFFEE 🤦

pkg/k8s/watchers/endpoint.go Outdated Show resolved Hide resolved
pkg/option/config.go Outdated Show resolved Hide resolved
pkg/option/config.go Show resolved Hide resolved
pkg/option/config.go Outdated Show resolved Hide resolved
@aanm aanm removed their assignment Dec 7, 2020
tklauser
tklauser reviewed Dec 7, 2020
View reviewed changes
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

There is also the crd-wait-timeout in the operator which was deprecated in 325547f. This one could be removed as part of this PR as well.

@jibi
daemon: remove deprecated k8s-watcher-queue-size option
232c57c 
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@jibi
daemon: remove depreacted blacklist-conflicting-routes option
054138f 
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@jibi
daemon: remove deprecated device option
0ab0d8f 
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@jibi jibi force-pushed the pr/jibi/remove-depreacted-v1.10-cli-opts branch from 80a67db to 0ab0d8f Compare December 7, 2020 11:07
fristonio
fristonio approved these changes Dec 7, 2020
View reviewed changes
Copy link
Member

@fristonio fristonio left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks 🚀

@jibi jibi requested a review from a team as a code owner December 7, 2020 11:23
@maintainer-s-little-helper
Copy link

Commit 6b0862427e84b032fcd21d7f2abb7d35b4a5c3f8 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 the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 7, 2020
@jibi
operator: remove crd-wait-timeout deprecated option
0c6db25 
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@jibi jibi force-pushed the pr/jibi/remove-depreacted-v1.10-cli-opts branch from 6b08624 to 0c6db25 Compare December 7, 2020 11:24
@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 Dec 7, 2020
@jibi jibi requested a review from aanm December 7, 2020 11:25
@jibi jibi requested a review from christarazi December 7, 2020 11:26
qmonnet
qmonnet approved these changes Dec 7, 2020
View reviewed changes
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.

Looks good, thank you!

tklauser
tklauser approved these changes Dec 7, 2020
View reviewed changes
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Thanks 🚀

@tklauser
Copy link
Member

tklauser commented Dec 7, 2020

test-me-please

@tklauser tklauser removed their assignment Dec 7, 2020
brb
brb approved these changes Dec 7, 2020
View reviewed changes
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jibi
Copy link
Member Author

jibi commented Dec 7, 2020

Looks like K8s-1.19-kernel-4.9 is failing due to a known flake: #13071
I'll restart the test

@jibi
Copy link
Member Author

jibi commented Dec 7, 2020

retest-4.9

@jibi
Copy link
Member Author

jibi commented Dec 7, 2020

retest-net-next

christarazi
christarazi approved these changes Dec 7, 2020
View reviewed changes
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

LGTM

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 7, 2020
@nebril nebril merged commit b027cc8 into master Dec 8, 2020
@nebril nebril deleted the pr/jibi/remove-depreacted-v1.10-cli-opts branch December 8, 2020 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brb brb brb approved these changes

@tklauser tklauser tklauser approved these changes

@christarazi christarazi christarazi approved these changes

@aanm aanm aanm approved these changes

@qmonnet qmonnet qmonnet approved these changes

@fristonio fristonio fristonio approved these changes

Assignees

@brb brb

@christarazi christarazi

@aanm aanm

Labels
area/cli Impacts the command line interface of any command in the repository. area/daemon Impacts operation of the Cilium daemon. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.10.0
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@jibi @tklauser @brb @christarazi @aanm @qmonnet @fristonio @nebril