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

Promote KEP-4292 custom profiling in kubectl debug to GA #4824

Merged
merged 3 commits into from
Oct 7, 2024
Merged

Promote KEP-4292 custom profiling in kubectl debug to GA #4824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
54e67eb
Promote KEP-4292 custom profiling in kubectl debug to GA
ardaguclu Sep 5, 2024
482b6c5
Apply review suggestions
ardaguclu Sep 30, 2024
40af3c3
Fix the feature gate removal release
ardaguclu Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions keps/prod-readiness/sig-cli/4292.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ alpha:
approver: "@johnbelamaric"
beta:
approver: "@johnbelamaric"
stable:
approver: "@johnbelamaric"
21 changes: 18 additions & 3 deletions keps/sig-cli/4292-kubectl-debug-custom-profile/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ Items marked with (R) are required *prior to targeting to a milestone / release*
- [x] (R) Production readiness review completed
- [x] (R) Production readiness review approved
- [X] "Implementation History" section is up-to-date for milestone
- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes
- [X] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
ardaguclu marked this conversation as resolved.
Show resolved Hide resolved
- [X] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes

<!--
**Note:** This checklist is iterative and should be reviewed and updated every time this enhancement is being considered for a milestone.
Expand Down Expand Up @@ -399,6 +399,8 @@ New unit test cases will be added for the following scenarios:
- During the copying pod, custom profile is set and `--profile` is `general` and the output is expected
- During the node debugging, custom profile is set and `--profile` is `general` and the output is expected

- `k8s.io/kubernetes/vendor/k8s.io/kubectl/pkg/cmd/debug`: `30-09-2024` - `67.3%`

##### Integration tests

<!--
Expand All @@ -425,6 +427,10 @@ this value is in the pod spec.
- Send invalid custom profile json(not in corev1.Container type or completely invalid json) and assure that
the error message is correct.

integration tests (defined in https://k8s.io/kubernetes/test/cmd/debug.sh#L571-L661) are running in
https://storage.googleapis.com/k8s-triage/index.html?pr=1&job=pull-kubernetes-integration


##### e2e tests

<!--
Expand Down Expand Up @@ -518,7 +524,7 @@ in back-to-back releases.

#### GA

- Environment variable is removed altogether and feature is enabled and can not be disabled.
- Feature gate (i.e. `KUBECTL_DEBUG_CUSTOM_PROFILE`) is locked to true and will be removed in 1.34.
- e2e tests are implemented and enabled.

### Upgrade / Downgrade Strategy
Expand Down Expand Up @@ -941,8 +947,16 @@ For each of them, fill in the following information by copying the below templat
- Testing: Are there any tests for failure mode? If not, describe why.
-->

- invalid debug pod after invalid custom profiling
- Detection: Debug pod is not in running state or not having required privileges to debug smoothly
- Mitigations: Pod can be deleted and re-run after modifying the custom profile
- Diagnostics: Pod can't be attached or in not running state
- Testing: There are several unit tests are implemented in https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/debug/debug_test.go

###### What steps should be taken if SLOs are not being met to determine the problem?

Not applicable

## Implementation History

<!--
Expand All @@ -957,6 +971,7 @@ Major milestones might include:
-->
- 2023-10-13: Kep is proposed as alpha feature
- 2024-06-04: Kep is promoted to beta
- 2024-09-05: Kep is promoted to stable
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll drop comments here, b/c I can't do that in not-modified parts:

  • in ##### Unit tests section please update the coverage after the addition of the new test cases described there.
  • in ##### Integration tests link k8s-triage pointing to new tests.
  • in ##### e2e tests add links to newly added e2e, if there are some
  • in #### GA section, as mentioned below, we should state that the feature gate will be locked to true, and removed in n+2 releases, to follow what we're doing in case of regular feature gates, which we should follow as closely as possible.
  • Missing answer for 2 questions ###### What are other known failure modes? and ###### What steps should be taken if SLOs are not being met to determine the problem?. The first deserves an answer, the latter can be marked as NA.

Copy link
Member Author

Choose a reason for hiding this comment

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

in ##### Unit tests section please update the coverage after the addition of the new test cases described there.

Added latest 67.3% test coverage.

in ##### Integration tests link k8s-triage pointing to new tests.

Added. However, debug.sh tests are not shown in this link, so I had to give the integration test link.

in ##### e2e tests add links to newly added e2e, if there are some

This PR kubernetes/kubernetes#127187 will add this

in #### GA section, as mentioned below, we should state that the feature gate will be locked to true, and removed in n+2 releases, to follow what we're doing in case of regular feature gates, which we should follow as closely as possible.

That is good point, I tried to reflect this.

Missing answer for 2 questions ###### What are other known failure modes? and ###### What steps should be taken if SLOs are not being met to determine the problem?. The first deserves an answer, the latter can be marked as NA.

Tried to answer.

Copy link
Member Author

Choose a reason for hiding this comment

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

@soltysh I incorporated your suggestions. Please let me know what you think about the latest version. Thank you.


## Drawbacks

Expand Down
6 changes: 3 additions & 3 deletions keps/sig-cli/4292-kubectl-debug-custom-profile/kep.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ see-also:
- "/keps/sig-cli/1441-kubectl-debug"

# The target maturity stage in the current dev cycle for this KEP.
stage: alpha
stage: stable

# The most recent milestone for which work toward delivery of this KEP has been
# done. This can be the current (upcoming) milestone, if it is being actively
# worked on.
latest-milestone: "v1.31"
latest-milestone: "v1.32"

# The milestone at which this feature was, or is targeted to be, at each stage.
milestone:
alpha: "v1.30"
beta: "v1.31"
stable: "TBD"
stable: "1.32"

# The following PRR answers are required at alpha release
# List the feature gate name and the components for which it must be enabled
Expand Down