From 287c0d61fe2e0571fd25b185efb1c5c2ab882ece Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Tue, 21 Nov 2023 16:32:50 -0500 Subject: [PATCH] Test ctrrcfg uses *resource.Quantity Signed-off-by: Qi Wang --- ...ontainer_runtime_config_controller_test.go | 73 ++++++++++- .../container-runtime-config/helpers.go | 12 +- .../container-runtime-config/helpers_test.go | 122 ++++++++++++++++++ 3 files changed, 200 insertions(+), 7 deletions(-) diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go index cb438e89f7..a23be45609 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" + "github.com/clarketm/json" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/klog/v2" @@ -1243,9 +1244,9 @@ func TestContainerRuntimeConfigOptions(t *testing.T) { validPidsLimit int64 = 2048 validZerolimit int64 = 0 invalidNegLimit int64 = -10 + three = resource.MustParse("3k") + ten = resource.MustParse("10k") ) - three := resource.MustParse("3k") - ten := resource.MustParse("10k") failureTests := []struct { name string config *mcfgv1.ContainerRuntimeConfiguration @@ -1337,6 +1338,74 @@ func TestContainerRuntimeConfigOptions(t *testing.T) { } } +func TestMarshalResourceQuantityOptionsJSON(t *testing.T) { + var ( + validLogSizeMax = resource.MustParse("10k") + validOverlaySize = resource.MustParse("10G") + ) + + emptyValueTests := []struct { + name string + config *mcfgv1.ContainerRuntimeConfiguration + }{ + { + name: "valid log level, overlaySize/logsizeMax should not appear in json", + config: &mcfgv1.ContainerRuntimeConfiguration{ + LogLevel: "debug", + }, + }, + { + name: "valid value of default runtimeļ¼Œ overlaySize/logsizeMax should not appear in json", + config: &mcfgv1.ContainerRuntimeConfiguration{ + DefaultRuntime: "crun", + }, + }, + } + + successTests := []struct { + name string + config *mcfgv1.ContainerRuntimeConfiguration + expectStr string + }{ + { + name: "valid max log size should appear in json", + config: &mcfgv1.ContainerRuntimeConfiguration{ + LogSizeMax: &validLogSizeMax, + LogLevel: "debug", + }, + expectStr: "\"logSizeMax\":\"10k\"", + }, + { + name: "valid max overlay size should appear in json", + config: &mcfgv1.ContainerRuntimeConfiguration{ + OverlaySize: &validOverlaySize, + LogLevel: "debug", + }, + expectStr: "\"overlaySize\":\"10G\"", + }, + } + + // Successful Tests + for _, test := range successTests { + ctrcfg := newContainerRuntimeConfig(test.name, test.config, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "", "")) + data, err := json.Marshal(ctrcfg) + if err != nil { + t.Errorf("%s: failed with %v. should have succeeded", test.name, err) + } + require.Contains(t, string(data), test.expectStr) + } + + for _, test := range emptyValueTests { + ctrcfg := newContainerRuntimeConfig(test.name, test.config, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "", "")) + data, err := json.Marshal(ctrcfg) + if err != nil { + t.Errorf("%s: failed with %v. should have succeeded", test.name, err) + } + require.NotContains(t, string(data), "\"overlaySize\"", "\"overlaySize\"") + require.NotContains(t, string(data), "\"logSizeMax\"", "\"logSizeMax\"") + } +} + func getKey(config *mcfgv1.ContainerRuntimeConfig, t *testing.T) string { key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(config) if err != nil { diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index e3b54e935f..380be46e28 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -295,12 +295,14 @@ func updateStorageConfig(data []byte, internal *mcfgv1.ContainerRuntimeConfigura return nil, fmt.Errorf("error decoding crio config: %w", err) } - if internal.OverlaySize != nil && internal.OverlaySize.Value() < 0 { - return nil, fmt.Errorf("invalid overlaySize config %q: the overlaySize should be larger than 0", internal.OverlaySize.String()) - } + if internal.OverlaySize != nil { + if internal.OverlaySize.Value() < 0 { + return nil, fmt.Errorf("invalid overlaySize config %q: the overlaySize should be larger than 0", internal.OverlaySize.String()) + } - if internal.OverlaySize != nil && internal.OverlaySize.Value() != 0 { - tomlConf.Storage.Options.Size = internal.OverlaySize.String() + if internal.OverlaySize.Value() != 0 { + tomlConf.Storage.Options.Size = internal.OverlaySize.String() + } } var newData bytes.Buffer diff --git a/pkg/controller/container-runtime-config/helpers_test.go b/pkg/controller/container-runtime-config/helpers_test.go index 51918f6ecb..a2bdd6a121 100644 --- a/pkg/controller/container-runtime-config/helpers_test.go +++ b/pkg/controller/container-runtime-config/helpers_test.go @@ -12,10 +12,14 @@ import ( "github.com/containers/image/v5/pkg/sysregistriesv2" signature "github.com/containers/image/v5/signature" "github.com/containers/image/v5/types" + storageconfig "github.com/containers/storage/pkg/config" apicfgv1 "github.com/openshift/api/config/v1" + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" ) @@ -910,3 +914,121 @@ func TestGetValidBlockAndAllowedRegistries(t *testing.T) { }) } } + +func TestCreateCRIODropinFiles(t *testing.T) { + zeroLogSizeMax := resource.MustParse("0k") + validLogSizeMax := resource.MustParse("10G") + + // Test zero value of logSizeMax will not be applied + zeroValueTests := []struct { + name string + cfg *mcfgv1.ContainerRuntimeConfiguration + filepath string + }{ + { + name: "01-ctrcfg-logSizeMax will not be created if logSizeMax is zero", + cfg: &mcfgv1.ContainerRuntimeConfiguration{ + LogSizeMax: &zeroLogSizeMax, + }, + filepath: crioDropInFilePathLogSizeMax, + }, + } + + // Test valid value of logSizeMax will be applied to the drop-in file + validValueTests := []struct { + name string + cfg *mcfgv1.ContainerRuntimeConfiguration + filepath string + want []byte + }{ + { + name: "01-ctrcfg-logSizeMax created for valid logSizeMax", + cfg: &mcfgv1.ContainerRuntimeConfiguration{ + LogSizeMax: &validLogSizeMax, + }, + filepath: crioDropInFilePathLogSizeMax, + want: []byte(`[crio] + [crio.runtime] + log_size_max = 10000000000 +`), + }, + } + + for _, test := range zeroValueTests { + ctrcfg := newContainerRuntimeConfig(test.name, test.cfg, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "", "")) + files := createCRIODropinFiles(ctrcfg) + for _, file := range files { + if file.filePath == test.filepath { + t.Errorf("%s: failed. should not have created dropin file", test.name) + } + } + } + + for _, test := range validValueTests { + ctrcfg := newContainerRuntimeConfig(test.name, test.cfg, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "", "")) + files := createCRIODropinFiles(ctrcfg) + for _, file := range files { + if file.filePath == test.filepath { + require.Equal(t, test.want, file.data, "createCRIODropinFiles() Diff, want %v, got %v", test.want, string(file.data)) + } + } + } +} + +func TestUpdateStorageConfig(t *testing.T) { + templateStorageConfig := tomlConfigStorage{} + buf := bytes.Buffer{} + err := toml.NewEncoder(&buf).Encode(templateStorageConfig) + require.NoError(t, err) + templateBytes := buf.Bytes() + + zeroOverLayerSize := resource.MustParse("0k") + validOverLaySize := resource.MustParse("10G") + + tests := []struct { + name string + cfg *mcfgv1.ContainerRuntimeConfiguration + want tomlConfigStorage + }{ + { + name: "not apply zero value of overlaySize", + cfg: &mcfgv1.ContainerRuntimeConfiguration{ + OverlaySize: &zeroOverLayerSize, + }, + want: tomlConfigStorage{}, + }, + { + name: "apply valid overlaySize", + cfg: &mcfgv1.ContainerRuntimeConfiguration{ + OverlaySize: &validOverLaySize, + }, + want: tomlConfigStorage{ + Storage: struct { + Driver string "toml:\"driver\"" + RunRoot string "toml:\"runroot\"" + GraphRoot string "toml:\"graphroot\"" + Options struct{ storageconfig.OptionsConfig } "toml:\"options\"" + }{ + Options: struct{ storageconfig.OptionsConfig }{ + storageconfig.OptionsConfig{ + Size: "10G", + }, + }, + }, + }, + }, + } + + for _, test := range tests { + got, err := updateStorageConfig(templateBytes, test.cfg) + require.NoError(t, err) + gotConf := tomlConfigStorage{} + if _, err := toml.Decode(string(got), &gotConf); err != nil { + t.Errorf("error unmarshalling result: %v", err) + return + } + if !reflect.DeepEqual(gotConf, test.want) { + t.Errorf("%s: failed. got %v, want %v", test.name, got, test.want) + } + } +}