feat: features metadata

[mlavacca]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?:

Features have been re-arranged to a struct containing metadata and feature Name. The API Channel has been set as a field for such a struct.

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[candita]

@mlavacca would you mind adding a docs update to the description on conformance testing so users know what they are getting themselves into with this change? Looks like every time you add a new conformance test there are quite a few places to change.

Since there's no GEP - why did you decide to make changes for all features and not just for experimental features?
For reference on my query - I solved a similar problem just by adding a new set to pkg/features/features.go, called ExperimentalPolicyFeatures, in #3212.

And my complaint - every time we make sweeping changes to conformance testing, we lose the skills and attention of people with experience using the previous implementation, thence potentially losing that team/skillset.

[mlavacca]

@mlavacca would you mind adding a docs update to the description on conformance testing so users know what they are getting themselves into with this change? Looks like every time you add a new conformance test there are quite a few places to change.

Not really. This PR adds some metadata to the features but does not introduce any change in how conformance tests are written. In order to create a new conformance test, we just need to set the feature name as part of the test. We need to perform a new step when defining new features (instead of just defining the feature name, we need to create a new feature struct as well). I'll ensure docs mention this aspect 👍 This is the first time we are trying to introduce a lifecycle concept on the features.

Since there's no GEP - why did you decide to make changes for all features and not just for experimental features? For reference on my query - I solved a similar problem just by adding a new set to pkg/features/features.go, called ExperimentalPolicyFeatures, in #3212.

It's not possible to make this change only for a subset of features. We are defining metadata on features, and I really think we need to do so for all our features, instead of having different mechanisms based on the feature maturity.

And my complaint - every time we make sweeping changes to conformance testing, we lose the skills and attention of people with experience using the previous implementation, thence potentially losing that team/skillset.

This is very related to the first point. If one wants to add a new conformance test to an already defined feature, nothing changes. In case a new feature has to be defined, the feature definition implies the creation of a struct instead of just the creation of a string. It does not seems that we are adding implementors-facing complexity to me.

[robscott]

Thanks @mlavacca!

[robscott]

The more I think about this, the more I think that we actually need two separate levels here:

1. The stability of the feature (the max release channel this has made it to)
2. The stability of the test (tests that are brand new should default to a lower level of stability than tests that have been around for a release cycle or two with multiple passing implementations)

I think it's useful to solve both of these, and this PR seems to be on a path to solve the first one. With that said, if we're agreeing that this PR is primarily targeting the first item, then I'd use the existing "Experimental" and "Standard" terminology for each feature.

Solving 2 may actually be easier. I think it just becomes a marker on tests, and implementations have the option to skip these tests when generating a report without any kind of penalty in their conformance report (or something along those lines).

[mlavacca]

Yep, I agree with the distinction and the fact that this PR is suitable to address the first point. As you pointed out, the 2nd point is easier, as it can be done via a specific field on the test struct.

I'll change the feature status names accordingly.

[mlavacca]

I've done the change and marked the PR as ready for review, PTAL :)

[robscott]

Thanks @mlavacca!

/lgtm

[shaneutt]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mlavacca, shaneutt

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:

• OWNERS [mlavacca,shaneutt]

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

[robscott]

/lgtm