Revert "Graduate Evented PLEG to Beta"

[pacoxu]

This reverts commit d971809.

What type of PR is this?

/kind bug
/kind cleanup

What this PR does / why we need it:

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/121345/pull-kubernetes-e2e-kind-beta-features/1745324395255042048 first pass of this job with disabling this FG EventedPLEG. See #121345 for more detailed CI status.

Which issue(s) this PR fixes:

revert as there is a known issue #121349 that will make static pod failed to start.

• We are working to fix the issue in [Failing Test] ci-kubernetes-e2e-kind-beta-features/pull-kubernetes-e2e-kind-beta-features failed for enabling some beta features(by default disabled) #121349.

It also makes the https://testgrid.k8s.io/sig-testing-kind#kind-master-beta failing for cluster init.

• and also https://prow.k8s.io/job-history/gs/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-e2e-kind-beta-features

Special notes for your reviewer:

• Kubelet Evented PLEG for Better Performance enhancements#3386
• Graduate Evented PLEG to Beta #115967 (1.27 promoted to beta but default false.)

Does this PR introduce a user-facing change?

Reverts the EventedPLEG feature (beta, but disabled by default) back to alpha for a known issue

[k8s-ci-robot]

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

[pacoxu]

/cc @pohly @aojea
https://kubernetes.slack.com/archives/C09QZ4DQB/p1704957019603369?thread_ts=1704859442.079249&cid=C09QZ4DQB discussions on slack

/assign @harche @mrunalp @dchen1107

[pacoxu]

/sig node

[pacoxu]

/test pull-kubernetes-e2e-kind-beta-features

[pohly]

During the discussion on Slack I was assuming that this must have been a change that went recently into master for 1.30 and some real bug.

However, now that I look at this I'm coming to a slightly different conclusion: as the original comment says, this is not a feature that can be enabled in all environments. That comment got lost when promoting to beta.

I think this is a misuse of the feature gate mechanism. If this cannot be enabled unconditionally, then there should be a separate kubelet (?) command line flag or configuration option for enabling it.

As it stands, this can only be promoted to GA (which implies removing the feature gate eventually!) if all CRI runtimes support this. Is that the goal? I haven't checked the KEP.

[aojea]

for one or other reason, this is clear no beta quality, I just found also that this issue was already reported in alpha an never addresses #121003

[pohly]

Do we have a precedence for downgrading a feature after it was released?

I agree that it should have remained alpha. I just want to understand whether downgrading really is an option.

[aojea]

I removed one entirely some time ago #121229

I don't know about precedence and this is better to consult as you say, @liggitt do you have any memories?

[harche]

I think this is a misuse of the feature gate mechanism. If this cannot be enabled unconditionally, then there should be a separate kubelet (?) command line flag or configuration option for enabling it.

Evented PLEG requires a CRI runtime that supports generating those events. KEP talks about falling back on Generic PLEG in case the kubelet with Evented PLEG is paired with the runtime that doesn't support generating the required events. So ideally, this feature should have worked unconditionally whether a CRI runtime supports Evented PLEG or not.

This has been handled in the code.

[harche]

@pacoxu's comment here indicates that PR fixed the issue caused by the Evented PLEG in the beta job, unless I misread it somehow.

After including the changes in that PR, is it failing with for some other reason?

[liggitt]

Do we have a precedence for downgrading a feature after it was released?

yes, when a feature was determined not to meet graduation requirements and cause regressions

ConfigMap rendering issue was found in the 1.25.0 release. When ConfigMaps get updated within the API, they do not get rendered to the resulting pod's filesystem by the Kubelet. The feature has been reverted to alpha in the 1.25.1 release.

[liggitt]

whatever change we make here should be consistently backported to 1.27/1.28/1.29

since the feature was never enabled by default, changing the state will not impact users who are currently explicitly enabling and using it (they can still do that if they want), but does accurately convey the stability level

[harche]

Although from the slack conversation it seems if the Evented PLEG feature is completely disabled then the beta job passes.

[pohly]

The job passes when it doesn't enable Evented PLEG. It should pass also when it is enabled because it should be safe to enable all beta features.

I'm not sure whether #122124 is a proper fix for this problem. I'll let people more familiar with the feature discuss that in that PR.

Even if that is fixed, is the quality and test coverage of Evented PLEG really good enough to call it beta?

The usual graduation criteria for beta are "e2e tests completed and enabled" and for beta, links are expected.

It looks like for PLEG, using the feature in a "real" cluster wasn't considered and not tested (https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/3386-kubelet-evented-pleg#graduation-criteria only mentions E2E node tests). E2E node testing is not sufficient.

Given that there were undetected problems, I'm 👍 for downgrading to alpha, updating the KEP with a more complete test plan, then upgrading to beta when known issues are fixed and test coverage is more complete.

/cc @aojea

[aojea]

This should have never go to beta without cluster test running and because it also had a critical issue open during 1.26 #121003 that turned out to be the problem why we are reverting now

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: c2255c9ae68d7b19d231bb0e1dc15dbeaa6e7f1e

[harche]

/test pull-kubernetes-e2e-gce-cos-alpha-features

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, mrunalp, pacoxu

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:

• pkg/features/OWNERS [mrunalp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mrunalp]

/hold

[mrunalp]

(Waiting for signal from pull-kubernetes-e2e-gce-cos-alpha-features)

[SergeyKanzhelev]

At a time of promotion we were debating if that was the right decision as we still need to work on extensive test coverage for the feature - including performance and consistency testing. The hope was that promoting to beta will simplify enabling it on certain environment and will allow to ensure a better coverage sooner. As we have not progressed much recently it may be a good idea to make it alpha again to ensure that the expectations for the feature quality are adequate. So I am supportive of this change.

/assign @dchen1107

[mrunalp]

/hold cancel

[liggitt]

Thanks for opening the cherry picks. Please copy the release note from here to those PRs as well, and open a PR to update the state and milestones in the KEP

[pacoxu]

Thanks for opening the cherry picks. Please copy the release note from here to those PRs as well, and open a PR to update the state and milestones in the KEP

Release notes are updated. I opened kubernetes/enhancements#4400 to sync the status.