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.

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

Add validation to help users move from usePolicyConfigMap #16391

Merged
merged 1 commit into from Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion k8s/crds/kops.k8s.io_clusters.yaml
Expand Up @@ -3872,7 +3872,8 @@ spec:
type: string
usePolicyConfigMap:
description: UsePolicyConfigMap enable setting the scheduler policy
from a configmap
from a configmap Deprecated - use KubeSchedulerConfiguration
instead
type: boolean
type: object
kubelet:
Expand Down
4 changes: 0 additions & 4 deletions nodeup/pkg/model/kube_scheduler.go
Expand Up @@ -196,10 +196,6 @@ func (b *KubeSchedulerBuilder) buildPod(kubeScheduler *kops.KubeSchedulerConfig)
flags = append(flags, "--"+flag+"kubeconfig="+kubescheduler.KubeConfigPath)
}

if fi.ValueOf(kubeScheduler.UsePolicyConfigMap) {
hakman marked this conversation as resolved.
Show resolved Hide resolved
flags = append(flags, "--policy-configmap=scheduler-policy", "--policy-configmap-namespace=kube-system")
}

pod := &v1.Pod{
TypeMeta: metav1.TypeMeta{
APIVersion: "v1",
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/kops/componentconfig.go
Expand Up @@ -745,6 +745,7 @@ type KubeSchedulerConfig struct {
// LeaderElection defines the configuration of leader election client.
LeaderElection *LeaderElectionConfiguration `json:"leaderElection,omitempty"`
// UsePolicyConfigMap enable setting the scheduler policy from a configmap
// Deprecated - use KubeSchedulerConfiguration instead
UsePolicyConfigMap *bool `json:"usePolicyConfigMap,omitempty"`
// FeatureGates is set of key=value pairs that describe feature gates for alpha/experimental features.
FeatureGates map[string]string `json:"featureGates,omitempty" flag:"feature-gates"`
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/kops/v1alpha2/componentconfig.go
Expand Up @@ -751,6 +751,7 @@ type KubeSchedulerConfig struct {
// LeaderElection defines the configuration of leader election client.
LeaderElection *LeaderElectionConfiguration `json:"leaderElection,omitempty"`
// UsePolicyConfigMap enable setting the scheduler policy from a configmap
// Deprecated - use KubeSchedulerConfiguration instead
UsePolicyConfigMap *bool `json:"usePolicyConfigMap,omitempty"`
// FeatureGates is set of key=value pairs that describe feature gates for alpha/experimental features.
FeatureGates map[string]string `json:"featureGates,omitempty" flag:"feature-gates"`
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/kops/v1alpha3/componentconfig.go
Expand Up @@ -742,6 +742,7 @@ type KubeSchedulerConfig struct {
// LeaderElection defines the configuration of leader election client.
LeaderElection *LeaderElectionConfiguration `json:"leaderElection,omitempty"`
// UsePolicyConfigMap enable setting the scheduler policy from a configmap
// Deprecated - use KubeSchedulerConfiguration instead
UsePolicyConfigMap *bool `json:"usePolicyConfigMap,omitempty"`
// FeatureGates is set of key=value pairs that describe feature gates for alpha/experimental features.
FeatureGates map[string]string `json:"featureGates,omitempty" flag:"feature-gates"`
Expand Down
16 changes: 16 additions & 0 deletions pkg/apis/kops/validation/validation.go
Expand Up @@ -134,6 +134,10 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
allErrs = append(allErrs, validateKubeControllerManager(spec.KubeControllerManager, c, fieldPath.Child("kubeControllerManager"), strict)...)
}

if spec.KubeScheduler != nil {
allErrs = append(allErrs, validateKubeScheduler(spec.KubeScheduler, c, fieldPath.Child("kubeScheduler"), strict)...)
}

if spec.KubeProxy != nil {
allErrs = append(allErrs, validateKubeProxy(spec.KubeProxy, fieldPath.Child("kubeProxy"))...)
}
Expand Down Expand Up @@ -842,6 +846,18 @@ func validateKubeControllerManager(v *kops.KubeControllerManagerConfig, c *kops.
return allErrs
}

func validateKubeScheduler(v *kops.KubeSchedulerConfig, c *kops.Cluster, fldPath *field.Path, strict bool) field.ErrorList {
allErrs := field.ErrorList{}

// We aren't aiming to do comprehensive validation, but we can add some best-effort validation where it helps guide users.
// Users reported encountered this in #16388
if v.UsePolicyConfigMap != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("usePolicyConfigMap"), "usePolicyConfigMap is deprecated, use KubeSchedulerConfiguration"))
hakman marked this conversation as resolved.
Show resolved Hide resolved
}

return allErrs
}

func validateKubeProxy(k *kops.KubeProxyConfig, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

Expand Down
46 changes: 1 addition & 45 deletions pkg/model/components/kubescheduler_test.go
Expand Up @@ -19,29 +19,12 @@ package components
import (
"testing"

api "k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops/util"
"k8s.io/kops/pkg/assets"
"k8s.io/kops/util/pkg/vfs"
)

func buildSchedulerConfigMapCluster(version string) *api.Cluster {
usePolicyConfigMap := true

return &api.Cluster{
Spec: api.ClusterSpec{
CloudProvider: api.CloudProviderSpec{
AWS: &api.AWSSpec{},
},
KubernetesVersion: version,
KubeScheduler: &api.KubeSchedulerConfig{
UsePolicyConfigMap: &usePolicyConfigMap,
},
},
}
}

func Test_Build_Scheduler_Without_PolicyConfigMap(t *testing.T) {
func Test_Build_Scheduler(t *testing.T) {
versions := []string{"v1.6.0", "v1.6.4", "v1.7.0", "v1.7.4"}

for _, v := range versions {
Expand Down Expand Up @@ -69,30 +52,3 @@ func Test_Build_Scheduler_Without_PolicyConfigMap(t *testing.T) {
}
}
}

func Test_Build_Scheduler_PolicyConfigMap_Supported_Version(t *testing.T) {
versions := []string{"v1.9.0", "v1.10.5", "v1.18.0"}

for _, v := range versions {

c := buildSchedulerConfigMapCluster(v)
b := assets.NewAssetBuilder(vfs.Context, c.Spec.Assets, c.Spec.KubernetesVersion, false)

version, err := util.ParseKubernetesVersion(v)
if err != nil {
t.Fatalf("unexpected error from ParseKubernetesVersion %s: %v", v, err)
}

ks := &KubeSchedulerOptionsBuilder{
&OptionsContext{
AssetBuilder: b,
KubernetesVersion: *version,
},
}

err = ks.BuildOptions(&c.Spec)
if err != nil {
t.Fatalf("unexpected error from BuildOptions %s: %v", v, err)
}
}
}

This file was deleted.

Expand Up @@ -1212,18 +1212,6 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.CloudupModelBuilderContext)
}
}

if b.Cluster.Spec.KubeScheduler.UsePolicyConfigMap != nil {
key := "scheduler.addons.k8s.io"
version := "1.7.0"
location := key + "/v" + version + ".yaml"

addons.Add(&channelsapi.AddonSpec{
Name: fi.PtrTo(key),
Selector: map[string]string{"k8s-addon": key},
Manifest: fi.PtrTo(location),
})
}

serviceAccounts := make(map[string]iam.Subject)

if b.Cluster.Spec.GetCloudProvider() == kops.CloudProviderAWS && b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer != nil {
Expand Down