[sample-apiserver] Fix: Use Correct Effective Version for kube #125941

fxierh · 2024-07-07T18:12:37Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Set effective kube version to default version + 1 minor version so the sample-apiserver could be deployed without the following error:

$ k logs wardle-server-65dbc6f5d-9pwv4 -n wardle
Defaulted container "wardle-server" out of: wardle-server, etcd
I0707 11:16:37.994750       1 registry.go:379] setting wardle:feature gate emulation version to 1.2
I0707 11:16:37.997302       1 registry.go:379] setting kube:feature gate emulation version to 1.32
I0707 11:16:37.997370       1 feature_gate.go:522] set feature gate emulationVersion to 1.32
I0707 11:16:37.998443       1 plugins.go:83] "Registered admission plugin" plugin="BanFlunder"
E0707 11:16:38.001481       1 run.go:72] "command failed" err="emulation version 1.32 is not between [1.30, 1.31.0]"

Which issue(s) this PR fixes:

Fixes #125938

k8s-ci-robot · 2024-07-07T18:12:45Z

Hi @fxierh. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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-sigs/prow repository.

pacoxu · 2024-07-08T07:50:07Z

This was added in #122891.

403301b#diff-3bae8ac9aa637addfd0b1c5db124f693550d913d85868c2b8c676afa87abbc6eR120.

/cc @siyuanfoundation @jpbetz
/ok-to-test

siyuanfoundation · 2024-07-08T16:18:00Z

/lgtm Thanks!

fxierh · 2024-07-08T19:05:55Z

/release-note-none

cici37 · 2024-07-09T20:20:32Z

/assign @siyuanfoundation Thanks :)
/triage accepted

fxierh · 2024-07-09T20:33:03Z

@jpbetz

Could you please take a look when you have a chance? Thank you!

jpbetz · 2024-07-11T15:48:34Z

/approve

test/integration/examples/apiserver_test.go

deads2k · 2024-07-23T20:13:57Z

test/integration/examples/apiserver_test.go

-	testServer := kastesting.StartTestServerOrDie(t, &kastesting.TestServerInstanceOptions{EnableCertAuth: true, BinaryVersion: "1.32"}, nil, framework.SharedEtcd())
+	// use DefaultKubeBinaryVersion - 1 minor for kube binary version, and 1.1 for wardle binary version so that we can test both N and N-1 wardle emulation versions.
+	binaryVersion := version.MustParse(baseversion.DefaultKubeBinaryVersion).OffsetMinor(-1)
+	wardleBinaryVersion := "1.1"


didn't expect this change

This is because if we use 1.2 and kube 1.31, we cannot test N-1 emu version, since setting kube emu version to 1.30 is not allowed for this release, that would be fixed in the next release.

Signed-off-by: Siyuan Zhang <sizhang@google.com> Signed-off-by: Feilian Xie <fxie@redhat.com> Co-authored-by: Feilian Xie <fxie@redhat.com>

Signed-off-by: Siyuan Zhang <sizhang@google.com>

fxierh · 2024-07-24T15:42:25Z

/milestone v1.31

k8s-ci-robot · 2024-07-24T15:42:27Z

@fxierh: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone v1.31

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-sigs/prow repository.

…ithout Wardle feature gate. Signed-off-by: Siyuan Zhang <sizhang@google.com>

fxierh · 2024-07-25T18:19:31Z

/test pull-kubernetes-e2e-gce

neoaggelos · 2024-07-26T06:36:45Z

/milestone v1.31

deads2k · 2024-07-26T15:32:04Z

staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go

@@ -177,7 +207,7 @@ func (o *WardleServerOptions) Config() (*apiserver.Config, error) {
 	serverConfig.OpenAPIV3Config.Info.Title = "Wardle"
 	serverConfig.OpenAPIV3Config.Info.Version = "0.1"

-	serverConfig.FeatureGate = utilversion.DefaultComponentGlobalsRegistry.FeatureGateFor(apiserver.WardleComponentName)
+	serverConfig.FeatureGate = utilversion.DefaultComponentGlobalsRegistry.FeatureGateFor(utilversion.DefaultKubeComponent)


This is indicative of a capability gap. A server extending our k8s.io/apiserver library needs a way to wire the custom FeatureGates that are wired to the command line flags. This suggests that the ServerConfig.FeatureGate needs to be a type that has func FeatureGateFor(component string) featuregate.FeatureGate from ComponentGlobalsRegistry.

I see this as a bug, not existential for the feature. Please open an issue and link it to the KEP issue so we dont' lose track.

opened #126393.

deads2k · 2024-07-26T15:40:21Z

test/integration/examples/apiserver_test.go

-		testAggregatedAPIServer(t, true, "1.1")
+	// Testing emulation version N-1, BanFlunder default=false in 1.0
+	t.Run("WithoutWardleFeatureGateAtV1.0", func(t *testing.T) {
+		testAggregatedAPIServer(t, false, false, "1.1", "1.0")


to avoid conflicts between the kube-apiserver featuregtes and the wardleapiserver featuregates are we better off directly doing CRUD against the wardle server instead of fully aggregating it for this particular set of tests? I think the testing suite woudl be more understandable then and it would ease future maintenance.

That's a good suggestion. Can we defer this to later since we do not have any time left in this code freeze?

That's a good suggestion. Can we defer this to later since we do not have any time left in this code freeze?

Yes, it is a test/bug fix.

deads2k · 2024-07-26T15:41:57Z

test/integration/examples/apiserver_test.go

 	t.Cleanup(func() { testServer.TearDownFn() })

+	_, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(


Given other changes, doesn't setting this here force the featuregate version to a particular level instead of allowing that level to be determined by the options passed to the wardle server, thus defeating the purpose of the integration test?

In a real use case, the binary version is always fixed. This setting forces the binary version to test in the integration test. This is temporary for 1.31 release, and can be removed in 1.32.

deads2k · 2024-07-26T17:36:44Z

/lgtm

k8s-ci-robot · 2024-07-26T17:36:51Z

LGTM label has been added.

Git tree hash: 3f8a33a46ad3cfc16a2fb6e1a17bd82ab5f4a615

deads2k · 2024-07-26T17:37:05Z

/milestone v1.31

deads2k · 2024-07-26T17:52:26Z

/approve

k8s-ci-robot · 2024-07-26T17:52:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, fxierh, jpbetz, siyuanfoundation, sttts

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:

~~staging/src/k8s.io/component-base/featuregate/OWNERS~~ [deads2k]
~~staging/src/k8s.io/sample-apiserver/OWNERS~~ [deads2k,jpbetz,sttts]
~~test/OWNERS~~ [deads2k,sttts]

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

…netes#125941) * Fix slice copy of VersionedSpecs in FeatureGate. Signed-off-by: Siyuan Zhang <sizhang@google.com> * Update wardle to kube version mapping Signed-off-by: Siyuan Zhang <sizhang@google.com> Signed-off-by: Feilian Xie <fxie@redhat.com> Co-authored-by: Feilian Xie <fxie@redhat.com> * Add cap to wardleEmulationVersionToKubeEmulationVersion. Signed-off-by: Siyuan Zhang <sizhang@google.com> * Add integration test for default BanFlunder behavior in version 1.2 without Wardle feature gate. Signed-off-by: Siyuan Zhang <sizhang@google.com> --------- Signed-off-by: Siyuan Zhang <sizhang@google.com> Signed-off-by: Feilian Xie <fxie@redhat.com> Co-authored-by: Siyuan Zhang <sizhang@google.com>

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/bug Categorizes issue or PR as related to a bug. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 7, 2024

k8s-ci-robot requested a review from deads2k July 7, 2024 18:12

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jul 7, 2024

k8s-ci-robot requested a review from sttts July 7, 2024 18:12

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 7, 2024

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 7, 2024

fxierh changed the title ~~Use an effective version which corresponds to the default effective k…~~ [sample-apiserver] Fix: Use Correct Effective Version for Wardle Jul 7, 2024

k8s-ci-robot requested review from jpbetz and siyuanfoundation July 8, 2024 07:50

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 8, 2024

fxierh force-pushed the sample-apiserver-emulation-version-fix branch from bd66c3f to 665479c Compare July 8, 2024 12:09

fxierh changed the title ~~[sample-apiserver] Fix: Use Correct Effective Version for Wardle~~ [sample-apiserver] Fix: Use Correct Effective Version for kube Jul 8, 2024

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 8, 2024

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 9, 2024

siyuanfoundation approved these changes Jul 9, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2024

fxierh requested a review from deads2k July 23, 2024 19:22

deads2k reviewed Jul 23, 2024

View reviewed changes

test/integration/examples/apiserver_test.go Show resolved Hide resolved

deads2k reviewed Jul 23, 2024

View reviewed changes

test/integration/examples/apiserver_test.go Outdated Show resolved Hide resolved

deads2k reviewed Jul 23, 2024

View reviewed changes

siyuanfoundation and others added 2 commits July 24, 2024 14:26

Update wardle to kube version mapping

cbfc211

Signed-off-by: Siyuan Zhang <sizhang@google.com> Signed-off-by: Feilian Xie <fxie@redhat.com> Co-authored-by: Feilian Xie <fxie@redhat.com>

Add cap to wardleEmulationVersionToKubeEmulationVersion.

3a5e3f4

Signed-off-by: Siyuan Zhang <sizhang@google.com>

fxierh force-pushed the sample-apiserver-emulation-version-fix branch from 5a17d10 to 3a5e3f4 Compare July 24, 2024 06:27

Add integration test for default BanFlunder behavior in version 1.2 w…

481a929

…ithout Wardle feature gate. Signed-off-by: Siyuan Zhang <sizhang@google.com>

fxierh requested a review from deads2k July 26, 2024 05:37

k8s-ci-robot added this to the v1.31 milestone Jul 26, 2024

deads2k reviewed Jul 26, 2024

View reviewed changes

deads2k added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jul 26, 2024

k8s-ci-robot assigned deads2k Jul 26, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 26, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 26, 2024

k8s-ci-robot merged commit ebdca53 into kubernetes:master Jul 26, 2024
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sample-apiserver] Fix: Use Correct Effective Version for kube #125941

[sample-apiserver] Fix: Use Correct Effective Version for kube #125941

fxierh commented Jul 7, 2024 •

edited

Loading

k8s-ci-robot commented Jul 7, 2024

pacoxu commented Jul 8, 2024

siyuanfoundation commented Jul 8, 2024

fxierh commented Jul 8, 2024

cici37 commented Jul 9, 2024

fxierh commented Jul 9, 2024

jpbetz commented Jul 11, 2024

deads2k Jul 23, 2024

siyuanfoundation Jul 23, 2024

fxierh commented Jul 24, 2024

k8s-ci-robot commented Jul 24, 2024

fxierh commented Jul 25, 2024

neoaggelos commented Jul 26, 2024

deads2k Jul 26, 2024

siyuanfoundation Jul 26, 2024

deads2k Jul 26, 2024

siyuanfoundation Jul 26, 2024

deads2k Jul 26, 2024

deads2k Jul 26, 2024

siyuanfoundation Jul 26, 2024

deads2k commented Jul 26, 2024

k8s-ci-robot commented Jul 26, 2024

deads2k commented Jul 26, 2024

deads2k commented Jul 26, 2024

k8s-ci-robot commented Jul 26, 2024

		t.Cleanup(func() { testServer.TearDownFn() })

		_, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(

[sample-apiserver] Fix: Use Correct Effective Version for kube #125941

[sample-apiserver] Fix: Use Correct Effective Version for kube #125941

Conversation

fxierh commented Jul 7, 2024 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

k8s-ci-robot commented Jul 7, 2024

pacoxu commented Jul 8, 2024

siyuanfoundation commented Jul 8, 2024

fxierh commented Jul 8, 2024

cici37 commented Jul 9, 2024

fxierh commented Jul 9, 2024

jpbetz commented Jul 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fxierh commented Jul 24, 2024

k8s-ci-robot commented Jul 24, 2024

fxierh commented Jul 25, 2024

neoaggelos commented Jul 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Jul 26, 2024

k8s-ci-robot commented Jul 26, 2024

deads2k commented Jul 26, 2024

deads2k commented Jul 26, 2024

k8s-ci-robot commented Jul 26, 2024

fxierh commented Jul 7, 2024 •

edited

Loading