Skip to content

Commit

Permalink
pkg/controller: add support for KubeletConfig Drop-in dir
Browse files Browse the repository at this point in the history
  • Loading branch information
@sohankunkerkar
sohankunkerkar committed Jan 10, 2025
1 parent f478312 commit 8b0f801
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 77 deletions.
171 changes: 109 additions & 62 deletions pkg/controller/kubelet-config/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package kubeletconfig
import (
"bytes"
"context"
"encoding/json"
"fmt"
"os"
"strconv"
"strings"

ign3types "github.com/coreos/ignition/v2/config/v3_4/types"
"github.com/imdario/mergo"
osev1 "github.com/openshift/api/config/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -20,6 +21,7 @@ import (
"k8s.io/apimachinery/pkg/util/yaml"
"k8s.io/klog/v2"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
yamlv2 "sigs.k8s.io/yaml"

mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned"
Expand Down Expand Up @@ -81,6 +83,12 @@ func createNewKubeletLogLevelIgnition(level int32) *ign3types.File {
return &r
}

func createNewKubeletDropInDirConfigIgnition(dir string) *ign3types.File {
config := fmt.Sprintf("[Service]\nEnvironment=\"KUBELET_CONFIG_DROPIN_DIR=%s\"\n", dir)
r := ctrlcommon.NewIgnFileBytesOverwriting("/etc/systemd/system/kubelet.service.d/30-dropin-dir.conf", []byte(config))
return &r
}

func createNewKubeletIgnition(yamlConfig []byte) *ign3types.File {

r := ctrlcommon.NewIgnFileBytesOverwriting("/etc/kubernetes/kubelet.conf", yamlConfig)
Expand Down Expand Up @@ -358,38 +366,52 @@ func validateUserKubeletConfig(cfg *mcfgv1.KubeletConfig) error {
if cfg.Spec.LogLevel != nil && (*cfg.Spec.LogLevel < 1 || *cfg.Spec.LogLevel > 10) {
return fmt.Errorf("KubeletConfig's LogLevel is not valid [1,10]: %v", cfg.Spec.LogLevel)
}
if cfg.Spec.KubeletConfig == nil || cfg.Spec.KubeletConfig.Raw == nil {
return nil
// Ensure DropInConfigDirectory and DropInConfigFile are set consistently.
if cfg.Spec.DropInConfigDirectory == nil && cfg.Spec.DropInConfigFile != nil {
return fmt.Errorf("DropInConfigFile cannot be set without DropInDirectory")
}
kcDecoded, err := DecodeKubeletConfig(cfg.Spec.KubeletConfig.Raw)
if err != nil {
return fmt.Errorf("KubeletConfig could not be unmarshalled, err: %w", err)
if cfg.Spec.DropInConfigDirectory != nil && cfg.Spec.DropInConfigFile == nil {
return fmt.Errorf("DropInConfigFile cannot be empty if DropInDirectory is set")
}

// Check all the fields a user cannot set within the KubeletConfig CR.
// If a user were to set these values, the system may become unrecoverable
// (ie: not recover after a reboot).
// Therefore, if the KubeletConfig CR instance contains a non-zero or non-empty value
// for one of the following fields, the MCC will not apply the CR and error out instead.
if kcDecoded.CgroupDriver != "" {
return fmt.Errorf("KubeletConfiguration: cgroupDriver is not allowed to be set, but contains: %s", kcDecoded.CgroupDriver)
}
if len(kcDecoded.ClusterDNS) > 0 {
return fmt.Errorf("KubeletConfiguration: clusterDNS is not allowed to be set, but contains: %s", kcDecoded.ClusterDNS)
}
if kcDecoded.ClusterDomain != "" {
return fmt.Errorf("KubeletConfiguration: clusterDomain is not allowed to be set, but contains: %s", kcDecoded.ClusterDomain)
}
if len(kcDecoded.FeatureGates) > 0 {
return fmt.Errorf("KubeletConfiguration: featureGates is not allowed to be set, but contains: %v", kcDecoded.FeatureGates)
if cfg.Spec.DropInConfigDirectory == nil && cfg.Spec.DropInConfigFile == nil {
return nil
}
if kcDecoded.StaticPodPath != "" {
return fmt.Errorf("KubeletConfiguration: staticPodPath is not allowed to be set, but contains: %s", kcDecoded.StaticPodPath)
if cfg.Spec.KubeletConfig == nil || cfg.Spec.KubeletConfig.Raw == nil {
return fmt.Errorf("DropInConfig cannot be empty")
}
if kcDecoded.SystemReserved != nil && len(kcDecoded.SystemReserved) > 0 &&
cfg.Spec.AutoSizingReserved != nil && *cfg.Spec.AutoSizingReserved {
return fmt.Errorf("KubeletConfiguration: autoSizingReserved and systemdReserved cannot be set together")

if cfg.Spec.KubeletConfig.Raw != nil {
kcDecoded, err := DecodeKubeletConfig(cfg.Spec.KubeletConfig.Raw)
if err != nil {
return fmt.Errorf("KubeletConfig in DropInConfig %s could not be unmarshalled, err: %w", *cfg.Spec.DropInConfigFile, err)
}

// Check all the fields a user cannot set within the KubeletConfig CR.
// If a user were to set these values, the system may become unrecoverable
// (ie: not recover after a reboot).
// Therefore, if the KubeletConfig CR instance contains a non-zero or non-empty value
// for one of the following fields, the MCC will not apply the CR and error out instead.
if kcDecoded.CgroupDriver != "" {
return fmt.Errorf("KubeletConfiguration: cgroupDriver is not allowed to be set, but contains: %s", kcDecoded.CgroupDriver)
}
if len(kcDecoded.ClusterDNS) > 0 {
return fmt.Errorf("KubeletConfiguration: clusterDNS is not allowed to be set, but contains: %s", kcDecoded.ClusterDNS)
}
if kcDecoded.ClusterDomain != "" {
return fmt.Errorf("KubeletConfiguration: clusterDomain is not allowed to be set, but contains: %s", kcDecoded.ClusterDomain)
}
if len(kcDecoded.FeatureGates) > 0 {
return fmt.Errorf("KubeletConfiguration: featureGates is not allowed to be set, but contains: %v", kcDecoded.FeatureGates)
}
if kcDecoded.StaticPodPath != "" {
return fmt.Errorf("KubeletConfiguration: staticPodPath is not allowed to be set, but contains: %s", kcDecoded.StaticPodPath)
}
if kcDecoded.SystemReserved != nil && len(kcDecoded.SystemReserved) > 0 &&
cfg.Spec.AutoSizingReserved != nil && *cfg.Spec.AutoSizingReserved {
return fmt.Errorf("KubeletConfiguration: autoSizingReserved and systemdReserved cannot be set together")
}
}

return nil
}

Expand Down Expand Up @@ -460,58 +482,47 @@ func kubeletConfigToIgnFile(cfg *kubeletconfigv1beta1.KubeletConfiguration) (*ig
}

// generateKubeletIgnFiles generates the Ignition files from the kubelet config
func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeConfig *kubeletconfigv1beta1.KubeletConfiguration) (*ign3types.File, *ign3types.File, *ign3types.File, error) {
func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeConfig *kubeletconfigv1beta1.KubeletConfiguration) (*ign3types.File, *ign3types.File, *ign3types.File, *ign3types.File, error) {
var (
kubeletIgnition *ign3types.File
logLevelIgnition *ign3types.File
autoSizingReservedIgnition *ign3types.File
dropInDirConfigIgnition *ign3types.File
)
userDefinedSystemReserved := make(map[string]string)

if kubeletConfig.Spec.KubeletConfig != nil && kubeletConfig.Spec.KubeletConfig.Raw != nil {
specKubeletConfig, err := DecodeKubeletConfig(kubeletConfig.Spec.KubeletConfig.Raw)
if kubeletConfig.Spec.DropInConfigDirectory != nil && kubeletConfig.Spec.DropInConfigFile != nil {
dropInDir := *kubeletConfig.Spec.DropInConfigDirectory
dropInConfig := *kubeletConfig.Spec.KubeletConfig
err := processDropInConfig(kubeletConfig, originalKubeConfig, userDefinedSystemReserved)
if err != nil {
return nil, nil, nil, fmt.Errorf("could not deserialize the new Kubelet config: %w", err)
}

if val, ok := specKubeletConfig.SystemReserved["memory"]; ok {
userDefinedSystemReserved["memory"] = val
delete(specKubeletConfig.SystemReserved, "memory")
return nil, nil, nil, nil, fmt.Errorf("could not process drop-in config: %w", err)
}

if val, ok := specKubeletConfig.SystemReserved["cpu"]; ok {
userDefinedSystemReserved["cpu"] = val
delete(specKubeletConfig.SystemReserved, "cpu")
// Decode JSON Raw into KubeletConfiguration
var kubeletConfigObj kubeletconfigv1beta1.KubeletConfiguration
err = json.Unmarshal(dropInConfig.Raw, &kubeletConfigObj)
if err != nil {
return nil, nil, nil, nil, fmt.Errorf("could not decode drop-in config JSON: %w", err)
}

if val, ok := specKubeletConfig.SystemReserved["ephemeral-storage"]; ok {
userDefinedSystemReserved["ephemeral-storage"] = val
delete(specKubeletConfig.SystemReserved, "ephemeral-storage")
// Convert the KubeletConfiguration into YAML
kubeletConfigYaml, err := yamlv2.Marshal(kubeletConfigObj)
if err != nil {
return nil, nil, nil, nil, fmt.Errorf("could not marshal KubeletConfiguration to YAML: %w", err)
}
// Write the specKubeletConfig to a file in the drop-in directory
dropInFilePath := fmt.Sprintf("%s/%s", dropInDir, *kubeletConfig.Spec.DropInConfigFile)

// FeatureGates must be set from the FeatureGate.
// Remove them here to prevent the specKubeletConfig merge overwriting them.
specKubeletConfig.FeatureGates = nil

// "protectKernelDefaults" is a boolean, optional field with `omitempty` json tag in the upstream kubelet configuration
// This field has been set to `true` by default in OCP recently
// As this field is an optional one with the above tag, it gets omitted when a user inputs it to `false`
// Reference: https://github.com/golang/go/issues/13284
// Adding a workaround to decide if the user has actually set the field to `false`
if strings.Contains(string(kubeletConfig.Spec.KubeletConfig.Raw), protectKernelDefaultsStr) {
originalKubeConfig.ProtectKernelDefaults = false
}
// Merge the Old and New
err = mergo.Merge(originalKubeConfig, specKubeletConfig, mergo.WithOverride)
if err != nil {
return nil, nil, nil, fmt.Errorf("could not merge original config and new config: %w", err)
// Write the YAML data to the specified file
if err := os.WriteFile(dropInFilePath, kubeletConfigYaml, 0644); err != nil {
return nil, nil, nil, nil, fmt.Errorf("could not write drop-in config to file: %w", err)
}
}

// Encode the new config into an Ignition File
kubeletIgnition, err := kubeletConfigToIgnFile(originalKubeConfig)
if err != nil {
return nil, nil, nil, fmt.Errorf("could not encode JSON: %w", err)
return nil, nil, nil, nil, fmt.Errorf("could not encode JSON: %w", err)
}

if kubeletConfig.Spec.LogLevel != nil {
Expand All @@ -523,6 +534,42 @@ func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeCo
if len(userDefinedSystemReserved) > 0 {
autoSizingReservedIgnition = createNewKubeletDynamicSystemReservedIgnition(nil, userDefinedSystemReserved)
}
if kubeletConfig.Spec.DropInConfigDirectory != nil {
dropInDirConfigIgnition = createNewKubeletDropInDirConfigIgnition(*kubeletConfig.Spec.DropInConfigDirectory)
}

return kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, dropInDirConfigIgnition, nil
}

return kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, nil
func processDropInConfig(
dropIn *mcfgv1.KubeletConfig,
originalKubeConfig *kubeletconfigv1beta1.KubeletConfiguration,
userDefinedSystemReserved map[string]string,
) error {
specKubeletConfig, err := DecodeKubeletConfig(dropIn.Spec.KubeletConfig.Raw)
if err != nil {
return fmt.Errorf("could not deserialize the new Kubelet config: %w", err)
}

for _, resource := range []string{"memory", "cpu", "ephemeral-storage"} {
if val, ok := specKubeletConfig.SystemReserved[resource]; ok {
userDefinedSystemReserved[resource] = val
delete(specKubeletConfig.SystemReserved, resource)
}
}

// FeatureGates must be set from the FeatureGate.
// Remove them here to prevent the specKubeletConfig merge overwriting them.
specKubeletConfig.FeatureGates = nil

// "protectKernelDefaults" is a boolean, optional field with `omitempty` json tag in the upstream kubelet configuration
// This field has been set to `true` by default in OCP recently
// As this field is an optional one with the above tag, it gets omitted when a user inputs it to `false`
// Reference: https://github.com/golang/go/issues/13284
// Adding a workaround to decide if the user has actually set the field to `false`
if strings.Contains(string(dropIn.Spec.KubeletConfig.Raw), protectKernelDefaultsStr) {
originalKubeConfig.ProtectKernelDefaults = false
}

return nil
}
5 changes: 4 additions & 1 deletion pkg/controller/kubelet-config/kubelet_config_bootstrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func RunKubeletBootstrap(templateDir string, kubeletConfigs []*mcfgv1.KubeletCon
originalKubeConfig.TLSCipherSuites = observedCipherSuites
}

kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig)
kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, dropInDirConfigIgnition, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig)
if err != nil {
return nil, err
}
Expand All @@ -71,6 +71,9 @@ func RunKubeletBootstrap(templateDir string, kubeletConfigs []*mcfgv1.KubeletCon
if kubeletIgnition != nil {
tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *kubeletIgnition)
}
if dropInDirConfigIgnition != nil {
tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *dropInDirConfigIgnition)
}

rawIgn, err := json.Marshal(tempIgnConfig)
if err != nil {
Expand Down
28 changes: 27 additions & 1 deletion pkg/controller/kubelet-config/kubelet_config_bootstrap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ package kubeletconfig

import (
"fmt"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
"k8s.io/utils/ptr"

configv1 "github.com/openshift/api/config/v1"
osev1 "github.com/openshift/api/config/v1"
Expand All @@ -33,12 +36,18 @@ func TestRunKubeletBootstrap(t *testing.T) {
if err != nil {
panic(err)
}
tempDir := t.TempDir()
kubeletConfDir := filepath.Join(tempDir, "kubelet.conf.d")
err = os.Mkdir(kubeletConfDir, 0755)
require.NoError(t, err, "Failed to create kubelet.conf.d directory")
// kubeletconfigs for master, worker， custom pool respectively
expectedMCNames := []string{"99-master-generated-kubelet", "99-worker-generated-kubelet", "99-custom-generated-kubelet"}
cfgs := []*mcfgv1.KubeletConfig{
{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-master"},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand All @@ -52,6 +61,8 @@ func TestRunKubeletBootstrap(t *testing.T) {
{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-worker"},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand All @@ -65,6 +76,8 @@ func TestRunKubeletBootstrap(t *testing.T) {
{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-custom", Labels: map[string]string{"node-role/custom": ""}},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand Down Expand Up @@ -106,7 +119,10 @@ func TestGenerateDefaultManagedKeyKubelet(t *testing.T) {
if err != nil {
panic(err)
}

tempDir := t.TempDir()
kubeletConfDir := filepath.Join(tempDir, "kubelet.conf.d")
err = os.Mkdir(kubeletConfDir, 0755)
require.NoError(t, err, "Failed to create kubelet.conf.d directory")
// valid case, only 1 kubeletconfig per pool
managedKeyExist := make(map[string]bool)
for _, tc := range []struct {
Expand All @@ -118,6 +134,8 @@ func TestGenerateDefaultManagedKeyKubelet(t *testing.T) {
&mcfgv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-default"},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand All @@ -130,6 +148,8 @@ func TestGenerateDefaultManagedKeyKubelet(t *testing.T) {
&mcfgv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-default"},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand All @@ -156,6 +176,8 @@ func TestGenerateDefaultManagedKeyKubelet(t *testing.T) {
&mcfgv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-default"},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand All @@ -169,6 +191,8 @@ func TestGenerateDefaultManagedKeyKubelet(t *testing.T) {
&mcfgv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-default"},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand All @@ -182,6 +206,8 @@ func TestGenerateDefaultManagedKeyKubelet(t *testing.T) {
&mcfgv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{Name: "kcfg-1"},
Spec: mcfgv1.KubeletConfigSpec{
DropInConfigDirectory: ptr.To(kubeletConfDir),
DropInConfigFile: ptr.To("10-kubelet.conf"),
KubeletConfig: &runtime.RawExtension{
Raw: kcRaw,
},
Expand Down
7 changes: 5 additions & 2 deletions pkg/controller/kubelet-config/kubelet_config_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
// Let's just pause for a bit here to let the renderer
// initialize them.
time.Sleep(updateDelay)
return fmt.Errorf("Pool %s is unconfigured, pausing %v for renderer to initialize", pool.Name, updateDelay)
return fmt.Errorf("pool %s is unconfigured, pausing %v for renderer to initialize", pool.Name, updateDelay)
}
role := pool.Name
// Get MachineConfig
Expand Down Expand Up @@ -645,7 +645,7 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
originalKubeConfig.TLSCipherSuites = observedCipherSuites
}

kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, err := generateKubeletIgnFiles(cfg, originalKubeConfig)
kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, dropInDirConfigIgnition, err := generateKubeletIgnFiles(cfg, originalKubeConfig)
if err != nil {
return ctrl.syncStatusOnly(cfg, err)
}
Expand Down Expand Up @@ -688,6 +688,9 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
if kubeletIgnition != nil {
tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *kubeletIgnition)
}
if dropInDirConfigIgnition != nil {
tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *dropInDirConfigIgnition)
}

rawIgn, err := json.Marshal(tempIgnConfig)
if err != nil {
Expand Down
Loading

0 comments on commit 8b0f801

Please sign in to comment.