Skip to content

Commit

Permalink
Merge pull request #14356 from hakman/automated-cherry-pick-of-#14333…
Browse files Browse the repository at this point in the history 
…-upstream-release-1.25

Automated cherry pick of #14333: Ensure kubelet configuration from IG takes precedence
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Sep 29, 2022
2 parents db1c9f3 + ad64cec commit 8098c61
Show file tree
Hide file tree
Showing 10 changed files with 239 additions and 55 deletions.
8 changes: 6 additions & 2 deletions k8s/crds/kops.k8s.io_clusters.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3489,7 +3489,9 @@ spec:
type: boolean
type: object
kubelet:
description: KubeletConfigSpec defines the kubelet configuration
description: Kubelet is the kubelet configuration for nodes not belonging
to the control plane. It can be overridden by the kubelet configuration
specified in the instance group.
properties:
allowPrivileged:
description: AllowPrivileged enables containers to request privileged
Expand Down Expand Up @@ -3917,7 +3919,9 @@ spec:
nodes
type: string
masterKubelet:
description: KubeletConfigSpec defines the kubelet configuration
description: MasterKubelet is the kubelet configuration for nodes
belonging to the control plane It can be overridden by the kubelet
configuration specified in the instance group.
properties:
allowPrivileged:
description: AllowPrivileged enables containers to request privileged
Expand Down
3 changes: 3 additions & 0 deletions nodeup/pkg/model/kubelet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package model
import (
"fmt"
"path/filepath"
"sort"
"testing"
"time"

Expand Down Expand Up @@ -137,6 +138,8 @@ func TestTaintsApplied(t *testing.T) {
}

func stringSlicesEqual(exp, other []string) bool {
sort.Sort(sort.StringSlice(exp))
sort.Sort(sort.StringSlice(other))
if exp == nil && other != nil {
return false
}
Expand Down
26 changes: 13 additions & 13 deletions pkg/apis/kops/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,15 @@ type ClusterSpec struct {
ExternalCloudControllerManager *CloudControllerManagerConfig `json:"cloudControllerManager,omitempty"`
KubeScheduler *KubeSchedulerConfig `json:"kubeScheduler,omitempty"`
KubeProxy *KubeProxyConfig `json:"kubeProxy,omitempty"`
Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"`
MasterKubelet *KubeletConfigSpec `json:"masterKubelet,omitempty"`
CloudConfig *CloudConfiguration `json:"cloudConfig,omitempty"`
ExternalDNS *ExternalDNSConfig `json:"externalDNS,omitempty"`
NTP *NTPConfig `json:"ntp,omitempty"`
// Kubelet is the kubelet configuration for nodes not belonging to the control plane.
// It can be overridden by the kubelet configuration specified in the instance group.
Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"`
// MasterKubelet is the kubelet configuration for nodes belonging to the control plane
// It can be overridden by the kubelet configuration specified in the instance group.
MasterKubelet *KubeletConfigSpec `json:"masterKubelet,omitempty"`
CloudConfig *CloudConfiguration `json:"cloudConfig,omitempty"`
ExternalDNS *ExternalDNSConfig `json:"externalDNS,omitempty"`
NTP *NTPConfig `json:"ntp,omitempty"`

// NodeTerminationHandler determines the node termination handler configuration.
NodeTerminationHandler *NodeTerminationHandlerConfig `json:"nodeTerminationHandler,omitempty"`
Expand Down Expand Up @@ -244,20 +248,16 @@ type CloudProviderSpec struct {
}

// AWSSpec configures the AWS cloud provider.
type AWSSpec struct {
}
type AWSSpec struct{}

// DOSpec configures the Digital Ocean cloud provider.
type DOSpec struct {
}
type DOSpec struct{}

// GCESpec configures the GCE cloud provider.
type GCESpec struct {
}
type GCESpec struct{}

// HetznerSpec configures the Hetzner cloud provider.
type HetznerSpec struct {
}
type HetznerSpec struct{}

type KarpenterConfig struct {
Enabled bool `json:"enabled,omitempty"`
Expand Down
14 changes: 9 additions & 5 deletions pkg/apis/kops/v1alpha2/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,15 @@ type ClusterSpec struct {
ExternalCloudControllerManager *CloudControllerManagerConfig `json:"cloudControllerManager,omitempty"`
KubeScheduler *KubeSchedulerConfig `json:"kubeScheduler,omitempty"`
KubeProxy *KubeProxyConfig `json:"kubeProxy,omitempty"`
Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"`
MasterKubelet *KubeletConfigSpec `json:"masterKubelet,omitempty"`
CloudConfig *CloudConfiguration `json:"cloudConfig,omitempty"`
ExternalDNS *ExternalDNSConfig `json:"externalDns,omitempty"`
NTP *NTPConfig `json:"ntp,omitempty"`
// Kubelet is the kubelet configuration for nodes not belonging to the control plane.
// It can be overridden by the kubelet configuration specified in the instance group.
Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"`
// MasterKubelet is the kubelet configuration for nodes belonging to the control plane
// It can be overridden by the kubelet configuration specified in the instance group.
MasterKubelet *KubeletConfigSpec `json:"masterKubelet,omitempty"`
CloudConfig *CloudConfiguration `json:"cloudConfig,omitempty"`
ExternalDNS *ExternalDNSConfig `json:"externalDns,omitempty"`
NTP *NTPConfig `json:"ntp,omitempty"`

// NodeTerminationHandler determines the cluster autoscaler configuration.
NodeTerminationHandler *NodeTerminationHandlerConfig `json:"nodeTerminationHandler,omitempty"`
Expand Down
26 changes: 13 additions & 13 deletions pkg/apis/kops/v1alpha3/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,15 @@ type ClusterSpec struct {
ExternalCloudControllerManager *CloudControllerManagerConfig `json:"cloudControllerManager,omitempty"`
KubeScheduler *KubeSchedulerConfig `json:"kubeScheduler,omitempty"`
KubeProxy *KubeProxyConfig `json:"kubeProxy,omitempty"`
Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"`
MasterKubelet *KubeletConfigSpec `json:"masterKubelet,omitempty"`
CloudConfig *CloudConfiguration `json:"cloudConfig,omitempty"`
ExternalDNS *ExternalDNSConfig `json:"externalDNS,omitempty"`
NTP *NTPConfig `json:"ntp,omitempty"`
// Kubelet is the kubelet configuration for nodes not belonging to the control plane.
// It can be overridden by the kubelet configuration specified in the instance group.
Kubelet *KubeletConfigSpec `json:"kubelet,omitempty"`
// MasterKubelet is the kubelet configuration for nodes belonging to the control plane
// It can be overridden by the kubelet configuration specified in the instance group.
MasterKubelet *KubeletConfigSpec `json:"masterKubelet,omitempty"`
CloudConfig *CloudConfiguration `json:"cloudConfig,omitempty"`
ExternalDNS *ExternalDNSConfig `json:"externalDNS,omitempty"`
NTP *NTPConfig `json:"ntp,omitempty"`

// NodeTerminationHandler determines the cluster autoscaler configuration.
NodeTerminationHandler *NodeTerminationHandlerConfig `json:"nodeTerminationHandler,omitempty"`
Expand Down Expand Up @@ -241,20 +245,16 @@ type CloudProviderSpec struct {
}

// AWSSpec configures the AWS cloud provider.
type AWSSpec struct {
}
type AWSSpec struct{}

// DOSpec configures the Digital Ocean cloud provider.
type DOSpec struct {
}
type DOSpec struct{}

// GCESpec configures the GCE cloud provider.
type GCESpec struct {
}
type GCESpec struct{}

// HetznerSpec configures the Hetzner cloud provider.
type HetznerSpec struct {
}
type HetznerSpec struct{}

type KarpenterConfig struct {
Enabled bool `json:"enabled,omitempty"`
Expand Down
22 changes: 22 additions & 0 deletions upup/pkg/fi/cloudup/populate_cluster_spec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,28 @@ func TestPopulateCluster_StorageDefault(t *testing.T) {
}
}

func TestPopulateCluster_EvictionHard(t *testing.T) {
cloud, c := buildMinimalCluster()

err := PerformAssignments(c, cloud)
if err != nil {
t.Fatalf("error from PerformAssignments: %v", err)
}

c.Spec.Kubelet = &kopsapi.KubeletConfigSpec{
EvictionHard: fi.String("memory.available<250Mi"),
}

full, err := mockedPopulateClusterSpec(c, cloud)
if err != nil {
t.Fatalf("Unexpected error from PopulateCluster: %v", err)
}

if fi.StringValue(full.Spec.Kubelet.EvictionHard) != "memory.available<250Mi" {
t.Fatalf("Unexpected StorageBackend: %v", *full.Spec.Kubelet.EvictionHard)
}
}

func build(c *kopsapi.Cluster) (*kopsapi.Cluster, error) {
cloud, err := BuildCloud(c)
if err != nil {
Expand Down
59 changes: 39 additions & 20 deletions upup/pkg/fi/cloudup/populate_instancegroup_spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"strings"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"

"k8s.io/kops/pkg/apis/kops"
Expand Down Expand Up @@ -74,7 +75,7 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup,
ig := &kops.InstanceGroup{}
reflectutils.JSONMergeStruct(ig, input)

spec := &ig.Spec
igSpec := &ig.Spec

// TODO: Clean up
if ig.IsMaster() {
Expand Down Expand Up @@ -223,39 +224,57 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup,
ig.Spec.Manager = kops.InstanceManagerCloudGroup
}

if spec.Kubelet == nil {
spec.Kubelet = &kops.KubeletConfigSpec{}
if igSpec.Kubelet == nil {
igSpec.Kubelet = &kops.KubeletConfigSpec{}
}

kubeletConfig := spec.Kubelet

// We include the NodeLabels in the userdata even for Kubernetes 1.16 and later so that
// rolling update will still replace nodes when they change.
kubeletConfig.NodeLabels = nodelabels.BuildNodeLabels(cluster, ig)

kubeletConfig.Taints = append(kubeletConfig.Taints, spec.Taints...)

var igKubeletConfig *kops.KubeletConfigSpec
// Start with the cluster kubelet config
if ig.IsMaster() {
reflectutils.JSONMergeStruct(kubeletConfig, cluster.Spec.MasterKubelet)

if cluster.Spec.MasterKubelet != nil {
igKubeletConfig = cluster.Spec.MasterKubelet.DeepCopy()
} else {
igKubeletConfig = &kops.KubeletConfigSpec{}
}
// A few settings in Kubelet override those in MasterKubelet. I'm not sure why.
if cluster.Spec.Kubelet != nil && cluster.Spec.Kubelet.AnonymousAuth != nil && !*cluster.Spec.Kubelet.AnonymousAuth {
kubeletConfig.AnonymousAuth = fi.Bool(false)
igKubeletConfig.AnonymousAuth = fi.Bool(false)
}
} else {
reflectutils.JSONMergeStruct(kubeletConfig, cluster.Spec.Kubelet)
if cluster.Spec.Kubelet != nil {
igKubeletConfig = cluster.Spec.Kubelet.DeepCopy()
} else {
igKubeletConfig = &kops.KubeletConfigSpec{}
}
}

// We include the NodeLabels in the userdata even for Kubernetes 1.16 and later so that
// rolling update will still replace nodes when they change.
igKubeletConfig.NodeLabels = nodelabels.BuildNodeLabels(cluster, ig)

useSecureKubelet := fi.BoolValue(igKubeletConfig.AnonymousAuth)

// While slices are overridden in most cases, taints are explicitly merged
taints := sets.NewString(igKubeletConfig.Taints...)
taints.Insert(igSpec.Taints...)
if ig.Spec.Kubelet != nil {
useSecureKubelet := fi.BoolValue(kubeletConfig.AnonymousAuth)
taints.Insert(igSpec.Kubelet.Taints...)
}
if cluster.Spec.Kubelet != nil {
taints.Insert(cluster.Spec.Kubelet.Taints...)
}
if ig.Spec.Kubelet != nil {
reflectutils.JSONMergeStruct(igKubeletConfig, ig.Spec.Kubelet)
}

reflectutils.JSONMergeStruct(kubeletConfig, spec.Kubelet)
igKubeletConfig.Taints = taints.List()

if useSecureKubelet {
kubeletConfig.AnonymousAuth = fi.Bool(false)
}
if useSecureKubelet {
igKubeletConfig.AnonymousAuth = fi.Bool(false)
}

ig.Spec.Kubelet = igKubeletConfig

return ig, nil
}

Expand Down
95 changes: 93 additions & 2 deletions upup/pkg/fi/cloudup/populate_instancegroup_spec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ func TestPopulateInstanceGroup_AddTaintsCollision3(t *testing.T) {
}

channel := &kopsapi.Channel{}

cloud, err := BuildCloud(cluster)
if err != nil {
t.Fatalf("error from BuildCloud: %v", err)
Expand All @@ -119,7 +118,99 @@ func TestPopulateInstanceGroup_AddTaintsCollision3(t *testing.T) {
t.Fatalf("error from PopulateInstanceGroupSpec: %v", err)
}
if len(output.Spec.Kubelet.Taints) != 2 {
t.Errorf("Expected only 2 taints, got %d", len(output.Spec.Kubelet.Taints))
t.Errorf("Expected 2 taints, got %d", len(output.Spec.Kubelet.Taints))
}
}

func TestPopulateInstanceGroup_EvictionHard(t *testing.T) {
_, cluster := buildMinimalCluster()
cluster.Spec.Kubelet = &kopsapi.KubeletConfigSpec{
EvictionHard: fi.String("memory.available<350Mi"),
}
input := buildMinimalNodeInstanceGroup()
input.Spec.Kubelet = &kopsapi.KubeletConfigSpec{
EvictionHard: fi.String("memory.available<250Mi"),
}

channel := &kopsapi.Channel{}

cloud, err := BuildCloud(cluster)
if err != nil {
t.Fatalf("error from BuildCloud: %v", err)
}
output, err := PopulateInstanceGroupSpec(cluster, input, cloud, channel)
if err != nil {
t.Fatalf("error from PopulateInstanceGroupSpec: %v", err)
}
if fi.StringValue(output.Spec.Kubelet.EvictionHard) != "memory.available<250Mi" {
t.Errorf("Unexpected value %v", fi.StringValue(output.Spec.Kubelet.EvictionHard))
}
}

func TestPopulateInstanceGroup_EvictionHard3(t *testing.T) {
_, cluster := buildMinimalCluster()
cluster.Spec.Kubelet = &kopsapi.KubeletConfigSpec{
EvictionHard: fi.String("memory.available<350Mi"),
}
input := buildMinimalMasterInstanceGroup("us-test-1")

channel := &kopsapi.Channel{}

cloud, err := BuildCloud(cluster)
if err != nil {
t.Fatalf("error from BuildCloud: %v", err)
}
output, err := PopulateInstanceGroupSpec(cluster, input, cloud, channel)
if err != nil {
t.Fatalf("error from PopulateInstanceGroupSpec: %v", err)
}
// There is no default EvictionHard
if fi.StringValue(output.Spec.Kubelet.EvictionHard) != "" {
t.Errorf("Unexpected value %v", fi.StringValue(output.Spec.Kubelet.EvictionHard))
}
}

func TestPopulateInstanceGroup_EvictionHard4(t *testing.T) {
_, cluster := buildMinimalCluster()
cluster.Spec.MasterKubelet = &kopsapi.KubeletConfigSpec{
EvictionHard: fi.String("memory.available<350Mi"),
}
input := buildMinimalMasterInstanceGroup("us-test-1")

channel := &kopsapi.Channel{}

cloud, err := BuildCloud(cluster)
if err != nil {
t.Fatalf("error from BuildCloud: %v", err)
}
output, err := PopulateInstanceGroupSpec(cluster, input, cloud, channel)
if err != nil {
t.Fatalf("error from PopulateInstanceGroupSpec: %v", err)
}
if fi.StringValue(output.Spec.Kubelet.EvictionHard) != "memory.available<350Mi" {
t.Errorf("Unexpected value %v", fi.StringValue(output.Spec.Kubelet.EvictionHard))
}
}

func TestPopulateInstanceGroup_EvictionHard2(t *testing.T) {
_, cluster := buildMinimalCluster()
input := buildMinimalNodeInstanceGroup()
input.Spec.Kubelet = &kopsapi.KubeletConfigSpec{
EvictionHard: fi.String("memory.available<250Mi"),
}

channel := &kopsapi.Channel{}

cloud, err := BuildCloud(cluster)
if err != nil {
t.Fatalf("error from BuildCloud: %v", err)
}
output, err := PopulateInstanceGroupSpec(cluster, input, cloud, channel)
if err != nil {
t.Fatalf("error from PopulateInstanceGroupSpec: %v", err)
}
if fi.StringValue(output.Spec.Kubelet.EvictionHard) != "memory.available<250Mi" {
t.Errorf("Unexpected value %v", fi.StringValue(output.Spec.Kubelet.EvictionHard))
}
}

Expand Down
1 change: 1 addition & 0 deletions util/pkg/reflectutils/walk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func IsMethodNotFound(err error) bool {

// JSONMergeStruct merges src into dest
// It uses a JSON marshal & unmarshal, so only fields that are JSON-visible will be copied
// If both source and destination has a value for a field, source takes presedence
func JSONMergeStruct(dest, src interface{}) {
// Not the most efficient approach, but simple & relatively well defined
j, err := json.Marshal(src)
Expand Down
Loading

0 comments on commit 8098c61

Please sign in to comment.