-
Notifications
You must be signed in to change notification settings - Fork 288
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.
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
Move the keep-alive
functionality out of the feature flag (+ deprecate the flag)
#1522
Move the keep-alive
functionality out of the feature flag (+ deprecate the flag)
#1522
Conversation
Hi @ditsuke. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/ok-to-test |
Can you squash and rebase the commits, this is ready to merge. Thanks for the quick turnaround on this one. |
553d353
to
3c6e06f
Compare
Done, thanks for reviewing |
/lgtm |
@geetikabatra: changing LGTM is restricted to collaborators In response to this:
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extremely sorry for this, I forgot that I had not posted this one. Just a couple of minor changes.
9d2098d
to
23f4633
Compare
deprecate(flags): enable-keep-alive flag The --enable-keep-alive flag is deprecated, since the functionality is now enabled by default. IN subsequent commits the keep-alive feature's dependence on this flag will be factored out. refactor: Decouple the keep-alive feature from the flag With this commit, the deprecated keep-alive feature flag is functionally removed. All functionality that previously relied on the flag's presence has been refactored to work without the flag. The CLI flag is still present but marked deprecated. chore: Remove keep-alive flag from manager deployment manifest The deprecated `--enable-keep-alive` is removed from the controller-manager's deployment manifest. chore: Update code comment in session/session Co-authored-by: Sagar Muchhal <8758225+srm09@users.noreply.github.com>
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: srm09 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
We now use the
keep-alive
functionality by default, eliminating the need for it to be an opt-in flag. Thus this PR:--enable-keep-alive
command line flag.keep-alive
functionality from the feature flag and removes the flag from the session-management feature config.Which issue(s) this PR fixes
Fixes #1520