From 8b3a93b843e05d7654d2b3888f02013b176a3a35 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 5 Feb 2025 15:39:04 -0800 Subject: [PATCH 1/3] fix: flagz endpoint to return parsed flags value --- cmd/kube-apiserver/app/options/completion.go | 2 +- cmd/kube-apiserver/app/options/options.go | 6 ++++++ cmd/kube-apiserver/app/options/options_test.go | 1 + cmd/kube-apiserver/app/server.go | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/cmd/kube-apiserver/app/options/completion.go b/cmd/kube-apiserver/app/options/completion.go index 0b83e79a9ed..8325b322c37 100644 --- a/cmd/kube-apiserver/app/options/completion.go +++ b/cmd/kube-apiserver/app/options/completion.go @@ -57,7 +57,7 @@ func (s *ServerRunOptions) Complete(ctx context.Context) (CompletedOptions, erro if err != nil { return CompletedOptions{}, err } - controlplane, err := s.Options.Complete(ctx, s.Flags(), []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes"}, []net.IP{apiServerServiceIP}) + controlplane, err := s.Options.Complete(ctx, *s.ParsedFlags, []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes"}, []net.IP{apiServerServiceIP}) if err != nil { return CompletedOptions{}, err } diff --git a/cmd/kube-apiserver/app/options/options.go b/cmd/kube-apiserver/app/options/options.go index ebed12af1d6..2ebe8a7ab59 100644 --- a/cmd/kube-apiserver/app/options/options.go +++ b/cmd/kube-apiserver/app/options/options.go @@ -41,6 +41,8 @@ type ServerRunOptions struct { CloudProvider *kubeoptions.CloudProviderOptions Extra + // ParsedFlags hold the parsed CLI flags. + ParsedFlags *cliflag.NamedFlagSets } type Extra struct { @@ -100,6 +102,9 @@ func NewServerRunOptions() *ServerRunOptions { // Flags returns flags for a specific APIServer by section name func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) { + if s.ParsedFlags != nil { + return *s.ParsedFlags + } s.Options.AddFlags(&fss) s.CloudProvider.AddFlags(fss.FlagSet("cloud provider")) @@ -156,5 +161,6 @@ func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) { "The number of apiservers running in the cluster, must be a positive number. (In use when --endpoint-reconciler-type=master-count is enabled.)") fs.MarkDeprecated("apiserver-count", "apiserver-count is deprecated and will be removed in a future version.") + s.ParsedFlags = &fss return fss } diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index 4da997c84bd..e3505b775e6 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/cmd/kube-apiserver/app/options/options_test.go @@ -334,6 +334,7 @@ func TestAddFlags(t *testing.T) { CloudConfigFile: "/cloud-config", CloudProvider: "azure", }, + ParsedFlags: s.ParsedFlags, } expected.Authentication.OIDC.UsernameClaim = "sub" diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 8aa05a4d8f8..101238b9e4c 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -124,6 +124,7 @@ cluster's shared state through which all other components interact.`, fs := cmd.Flags() namedFlagSets := s.Flags() + s.ParsedFlags = &namedFlagSets verflag.AddFlags(namedFlagSets.FlagSet("global")) globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name(), logs.SkipLoggingConfigurationFlags()) options.AddCustomGlobalFlags(namedFlagSets.FlagSet("generic")) From a0eb3acf30e187ec4cbd080a725563fa3252e361 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 5 Feb 2025 15:40:33 -0800 Subject: [PATCH 2/3] test: Add emulated-version flag verification in flagz test --- cmd/kube-apiserver/app/options/completion.go | 2 +- cmd/kube-apiserver/app/options/options.go | 6 --- .../app/options/options_test.go | 1 - cmd/kube-apiserver/app/server.go | 8 +++- cmd/kube-apiserver/app/testing/testserver.go | 8 +++- pkg/controlplane/apiserver/config_test.go | 3 +- pkg/controlplane/apiserver/options/options.go | 4 +- .../samples/generic/server/server.go | 3 +- .../generic/server/testing/testserver.go | 8 +++- .../controlplane/kube_apiserver_test.go | 44 +++++++++++++++++++ 10 files changed, 69 insertions(+), 18 deletions(-) diff --git a/cmd/kube-apiserver/app/options/completion.go b/cmd/kube-apiserver/app/options/completion.go index 8325b322c37..89e2dbf9e41 100644 --- a/cmd/kube-apiserver/app/options/completion.go +++ b/cmd/kube-apiserver/app/options/completion.go @@ -57,7 +57,7 @@ func (s *ServerRunOptions) Complete(ctx context.Context) (CompletedOptions, erro if err != nil { return CompletedOptions{}, err } - controlplane, err := s.Options.Complete(ctx, *s.ParsedFlags, []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes"}, []net.IP{apiServerServiceIP}) + controlplane, err := s.Options.Complete(ctx, []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes"}, []net.IP{apiServerServiceIP}) if err != nil { return CompletedOptions{}, err } diff --git a/cmd/kube-apiserver/app/options/options.go b/cmd/kube-apiserver/app/options/options.go index 2ebe8a7ab59..ebed12af1d6 100644 --- a/cmd/kube-apiserver/app/options/options.go +++ b/cmd/kube-apiserver/app/options/options.go @@ -41,8 +41,6 @@ type ServerRunOptions struct { CloudProvider *kubeoptions.CloudProviderOptions Extra - // ParsedFlags hold the parsed CLI flags. - ParsedFlags *cliflag.NamedFlagSets } type Extra struct { @@ -102,9 +100,6 @@ func NewServerRunOptions() *ServerRunOptions { // Flags returns flags for a specific APIServer by section name func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) { - if s.ParsedFlags != nil { - return *s.ParsedFlags - } s.Options.AddFlags(&fss) s.CloudProvider.AddFlags(fss.FlagSet("cloud provider")) @@ -161,6 +156,5 @@ func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) { "The number of apiservers running in the cluster, must be a positive number. (In use when --endpoint-reconciler-type=master-count is enabled.)") fs.MarkDeprecated("apiserver-count", "apiserver-count is deprecated and will be removed in a future version.") - s.ParsedFlags = &fss return fss } diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index e3505b775e6..4da997c84bd 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/cmd/kube-apiserver/app/options/options_test.go @@ -334,7 +334,6 @@ func TestAddFlags(t *testing.T) { CloudConfigFile: "/cloud-config", CloudProvider: "azure", }, - ParsedFlags: s.ParsedFlags, } expected.Authentication.OIDC.UsernameClaim = "sub" diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 101238b9e4c..444ef28c715 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -48,6 +48,8 @@ import ( "k8s.io/component-base/term" utilversion "k8s.io/component-base/version" "k8s.io/component-base/version/verflag" + zpagesfeatures "k8s.io/component-base/zpages/features" + "k8s.io/component-base/zpages/flagz" "k8s.io/klog/v2" aggregatorapiserver "k8s.io/kube-aggregator/pkg/apiserver" "k8s.io/kubernetes/cmd/kube-apiserver/app/options" @@ -124,7 +126,11 @@ cluster's shared state through which all other components interact.`, fs := cmd.Flags() namedFlagSets := s.Flags() - s.ParsedFlags = &namedFlagSets + if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) { + s.Flagz = flagz.NamedFlagSetsReader{ + FlagSets: namedFlagSets, + } + } verflag.AddFlags(namedFlagSets.FlagSet("global")) globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name(), logs.SkipLoggingConfigurationFlags()) options.AddCustomGlobalFlags(namedFlagSets.FlagSet("generic")) diff --git a/cmd/kube-apiserver/app/testing/testserver.go b/cmd/kube-apiserver/app/testing/testserver.go index 685047ac10e..ee7a8e52727 100644 --- a/cmd/kube-apiserver/app/testing/testserver.go +++ b/cmd/kube-apiserver/app/testing/testserver.go @@ -59,6 +59,8 @@ import ( featuregatetesting "k8s.io/component-base/featuregate/testing" logsapi "k8s.io/component-base/logs/api/v1" utilversion "k8s.io/component-base/version" + zpagesfeatures "k8s.io/component-base/zpages/features" + "k8s.io/component-base/zpages/flagz" "k8s.io/klog/v2" "k8s.io/kube-aggregator/pkg/apiserver" "k8s.io/kubernetes/pkg/features" @@ -213,7 +215,8 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions, s.GenericServerRunOptions.RequestTimeout = instanceOptions.RequestTimeout } - for _, f := range s.Flags().FlagSets { + namedFlagSets := s.Flags() + for _, f := range namedFlagSets.FlagSets { fs.AddFlagSet(f) } @@ -356,6 +359,9 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions, if err := fs.Parse(customFlags); err != nil { return result, err } + if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) { + s.Flagz = flagz.NamedFlagSetsReader{FlagSets: namedFlagSets} + } // the RequestHeader options pointer gets replaced in the case of EnableCertAuth override // and so flags are connected to a struct that no longer appears in the ServerOptions struct diff --git a/pkg/controlplane/apiserver/config_test.go b/pkg/controlplane/apiserver/config_test.go index 9f624cbcba7..b6d4f979927 100644 --- a/pkg/controlplane/apiserver/config_test.go +++ b/pkg/controlplane/apiserver/config_test.go @@ -25,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" apiserveroptions "k8s.io/apiserver/pkg/server/options" - cliflag "k8s.io/component-base/cli/flag" aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/controlplane/apiserver/options" @@ -47,7 +46,7 @@ func TestBuildGenericConfig(t *testing.T) { s.BindPort = ln.Addr().(*net.TCPAddr).Port opts.SecureServing = s - completedOptions, err := opts.Complete(context.TODO(), cliflag.NamedFlagSets{}, nil, nil) + completedOptions, err := opts.Complete(context.TODO(), nil, nil) if err != nil { t.Fatalf("Failed to complete apiserver options: %v", err) } diff --git a/pkg/controlplane/apiserver/options/options.go b/pkg/controlplane/apiserver/options/options.go index 2266be0cbed..28741c84c18 100644 --- a/pkg/controlplane/apiserver/options/options.go +++ b/pkg/controlplane/apiserver/options/options.go @@ -203,7 +203,7 @@ func (s *Options) AddFlags(fss *cliflag.NamedFlagSets) { "Path to socket where a external JWT signer is listening. This flag is mutually exclusive with --service-account-signing-key-file and --service-account-key-file. Requires enabling feature gate (ExternalServiceAccountTokenSigner)") } -func (o *Options) Complete(ctx context.Context, fss cliflag.NamedFlagSets, alternateDNS []string, alternateIPs []net.IP) (CompletedOptions, error) { +func (o *Options) Complete(ctx context.Context, alternateDNS []string, alternateIPs []net.IP) (CompletedOptions, error) { if o == nil { return CompletedOptions{completedOptions: &completedOptions{}}, nil } @@ -259,8 +259,6 @@ func (o *Options) Complete(ctx context.Context, fss cliflag.NamedFlagSets, alter } } - completed.Flagz = flagz.NamedFlagSetsReader{FlagSets: fss} - return CompletedOptions{ completedOptions: &completed, }, nil diff --git a/pkg/controlplane/apiserver/samples/generic/server/server.go b/pkg/controlplane/apiserver/samples/generic/server/server.go index cbc6d9e5633..be6af3f94d4 100644 --- a/pkg/controlplane/apiserver/samples/generic/server/server.go +++ b/pkg/controlplane/apiserver/samples/generic/server/server.go @@ -24,7 +24,6 @@ import ( "path/filepath" "github.com/spf13/cobra" - "github.com/spf13/pflag" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -86,7 +85,7 @@ APIs.`, ctx := genericapiserver.SetupSignalContext() - completedOptions, err := s.Complete(ctx, cliflag.NamedFlagSets{FlagSets: map[string]*pflag.FlagSet{"sample_generic_controlplane": fs}}, []string{}, []net.IP{}) + completedOptions, err := s.Complete(ctx, []string{}, []net.IP{}) if err != nil { return err } diff --git a/pkg/controlplane/apiserver/samples/generic/server/testing/testserver.go b/pkg/controlplane/apiserver/samples/generic/server/testing/testserver.go index f7dc5453589..16a0667711b 100644 --- a/pkg/controlplane/apiserver/samples/generic/server/testing/testserver.go +++ b/pkg/controlplane/apiserver/samples/generic/server/testing/testserver.go @@ -37,10 +37,13 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/storage/storagebackend" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" cliflag "k8s.io/component-base/cli/flag" logsapi "k8s.io/component-base/logs/api/v1" + zpagesfeatures "k8s.io/component-base/zpages/features" + "k8s.io/component-base/zpages/flagz" "k8s.io/klog/v2" controlplaneapiserver "k8s.io/kubernetes/pkg/controlplane/apiserver/options" "k8s.io/kubernetes/test/utils/ktesting" @@ -126,6 +129,9 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions, o := server.NewOptions() var fss cliflag.NamedFlagSets o.AddFlags(&fss) + if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) { + o.Flagz = flagz.NamedFlagSetsReader{FlagSets: fss} + } fs := pflag.NewFlagSet("test", pflag.PanicOnError) for _, f := range fss.FlagSets { @@ -164,7 +170,7 @@ func StartTestServer(t ktesting.TB, instanceOptions *TestServerInstanceOptions, o.Authentication.ServiceAccounts.Issuers = []string{"https://foo.bar.example.com"} o.Authentication.ServiceAccounts.KeyFiles = []string{saSigningKeyFile.Name()} - completedOptions, err := o.Complete(tCtx, fss, nil, nil) + completedOptions, err := o.Complete(tCtx, nil, nil) if err != nil { return result, fmt.Errorf("failed to set default ServerRunOptions: %w", err) } diff --git a/test/integration/controlplane/kube_apiserver_test.go b/test/integration/controlplane/kube_apiserver_test.go index 70148673e3e..9580d5bf821 100644 --- a/test/integration/controlplane/kube_apiserver_test.go +++ b/test/integration/controlplane/kube_apiserver_test.go @@ -28,6 +28,9 @@ import ( "time" "k8s.io/apiextensions-apiserver/test/integration/fixtures" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/component-base/zpages/features" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -127,6 +130,47 @@ func TestLivezAndReadyz(t *testing.T) { } } +func TestFlagz(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ComponentFlagz, true) + testServerFlags := append(framework.DefaultTestServerFlags(), "--emulated-version=1.32") + server := kubeapiservertesting.StartTestServerOrDie(t, nil, testServerFlags, framework.SharedEtcd()) + defer server.TearDownFn() + + client, err := kubernetes.NewForConfig(server.ClientConfig) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + res := client.CoreV1().RESTClient().Get().RequestURI("/flagz").Do(context.TODO()) + var status int + res.StatusCode(&status) + if status != http.StatusOK { + t.Fatalf("flagz/ should be healthy, got %v", status) + } + + expectedHeader := ` +kube-apiserver flags +Warning: This endpoint is not meant to be machine parseable, has no formatting compatibility guarantees and is for debugging purposes only.` + + raw, err := res.Raw() + if err != nil { + t.Fatal(err) + } + if !bytes.HasPrefix(raw, []byte(expectedHeader)) { + t.Fatalf("Header mismatch!\nExpected:\n%s\n\nGot:\n%s", expectedHeader, string(raw)) + } + found := false + for _, line := range strings.Split(string(raw), "\n") { + if strings.Contains(line, "emulated-version") && strings.Contains(line, "1.32") { + found = true + break + } + } + if !found { + t.Fatalf("Expected flag --emulated-version=[1.32] to be reflected in /flagz output, got:\n%s", string(raw)) + } +} + // TestOpenAPIDelegationChainPlumbing is a smoke test that checks for // the existence of some representative paths from the // apiextensions-server and the kube-aggregator server, both part of From 99eedea5ccafa8ad897b5d97ea9b13984c80d5e0 Mon Sep 17 00:00:00 2001 From: Richa Banker Date: Thu, 20 Feb 2025 19:32:05 -0800 Subject: [PATCH 3/3] Remove the feature-gate check before populating serverRunOptions.Flagz --- cmd/kube-apiserver/app/server.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 444ef28c715..7f88686ea71 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -48,7 +48,6 @@ import ( "k8s.io/component-base/term" utilversion "k8s.io/component-base/version" "k8s.io/component-base/version/verflag" - zpagesfeatures "k8s.io/component-base/zpages/features" "k8s.io/component-base/zpages/flagz" "k8s.io/klog/v2" aggregatorapiserver "k8s.io/kube-aggregator/pkg/apiserver" @@ -126,10 +125,8 @@ cluster's shared state through which all other components interact.`, fs := cmd.Flags() namedFlagSets := s.Flags() - if utilfeature.DefaultFeatureGate.Enabled(zpagesfeatures.ComponentFlagz) { - s.Flagz = flagz.NamedFlagSetsReader{ - FlagSets: namedFlagSets, - } + s.Flagz = flagz.NamedFlagSetsReader{ + FlagSets: namedFlagSets, } verflag.AddFlags(namedFlagSets.FlagSet("global")) globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name(), logs.SkipLoggingConfigurationFlags())