From 5b65660ebaa7d00fc702d5c913ec069aedb19fb5 Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Wed, 16 Oct 2024 21:36:17 -0700 Subject: [PATCH] Avoid error log when unmarshalling config for MC controller (#6744) The MultiClusterConfig struct does not have yaml tags, only json tags. Therefore, trying to unmarshal it as YAML (by calling yaml.UnmarshalLenient) will not work, and will show a log indicating that strict unmarshalling has failed. The resulting struct object will be all empty, since none of the fields can be decoded without a tag. A key observation is that calling yaml.UnmarshalLenient does not serve any purpose, as we are also calling runtime.DecodeInto (which will first convert the YAML data to JSON data, before unmarshalling). Hence we can just remove the call to yaml.UnmarshalLenient. We add a couple of unit tests, and in particular we validate that the default controller_manager_config.yaml config can be unmarshalled (strictly) to MultiClusterConfig. While this is slightly orthogonal to the issue being addressed here, it may help avoid issues in the future as it ensures that the default config is always in sync with the struct definition. Signed-off-by: Antonin Bas --- .../cmd/multicluster-controller/options.go | 22 +++++++++---------- .../multicluster-controller/options_test.go | 20 +++++++++++++++++ multicluster/embed.go | 19 ++++++++++++++++ 3 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 multicluster/embed.go diff --git a/multicluster/cmd/multicluster-controller/options.go b/multicluster/cmd/multicluster-controller/options.go index 17730f9089c..ebcced4c1ea 100644 --- a/multicluster/cmd/multicluster-controller/options.go +++ b/multicluster/cmd/multicluster-controller/options.go @@ -28,7 +28,6 @@ import ( mcsv1alpha1 "antrea.io/antrea/multicluster/apis/multicluster/v1alpha1" "antrea.io/antrea/multicluster/controllers/multicluster/common" - "antrea.io/antrea/pkg/util/yaml" ) type Options struct { @@ -123,25 +122,24 @@ func (o *Options) setDefaults() { } } -func (o *Options) loadConfigFromFile(multiclusterConfig *mcsv1alpha1.MultiClusterConfig) error { - data, err := os.ReadFile(o.configFile) - if err != nil { - return err - } +func (o *Options) loadConfig(data []byte, multiclusterConfig *mcsv1alpha1.MultiClusterConfig) error { codecs := serializer.NewCodecFactory(scheme) - if err := yaml.UnmarshalLenient(data, multiclusterConfig); err != nil { - return err - } - if err = runtime.DecodeInto(codecs.UniversalDecoder(), data, multiclusterConfig); err != nil { + if err := runtime.DecodeInto(codecs.UniversalDecoder(), data, multiclusterConfig); err != nil { return err } - if multiclusterConfig.Metrics.BindAddress != "" { o.options.Metrics.BindAddress = multiclusterConfig.Metrics.BindAddress } if multiclusterConfig.Health.HealthProbeBindAddress != "" { o.options.HealthProbeBindAddress = multiclusterConfig.Health.HealthProbeBindAddress } - return nil } + +func (o *Options) loadConfigFromFile(multiclusterConfig *mcsv1alpha1.MultiClusterConfig) error { + data, err := os.ReadFile(o.configFile) + if err != nil { + return err + } + return o.loadConfig(data, multiclusterConfig) +} diff --git a/multicluster/cmd/multicluster-controller/options_test.go b/multicluster/cmd/multicluster-controller/options_test.go index d053446330e..35acd575746 100644 --- a/multicluster/cmd/multicluster-controller/options_test.go +++ b/multicluster/cmd/multicluster-controller/options_test.go @@ -19,6 +19,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/yaml" + + "antrea.io/antrea/multicluster" + mcsv1alpha1 "antrea.io/antrea/multicluster/apis/multicluster/v1alpha1" ) func TestComplete(t *testing.T) { @@ -85,3 +89,19 @@ func TestComplete(t *testing.T) { }) } } + +func TestUnmarshalDefaultConfig(t *testing.T) { + configBytes := multicluster.DefaultControllerManagerConfigBytes + var multiclusterConfig mcsv1alpha1.MultiClusterConfig + // Note that we use UnmarshalStrict from k8s.io/apimachinery/pkg/util/yaml, which will first + // convert the YAML data to JSON. The mcsv1alpha1.MultiClusterConfig struct definition does + // not have yaml tags. + assert.NoError(t, yaml.UnmarshalStrict(configBytes, &multiclusterConfig), "Default config should unmarshal correctly") +} + +func TestLoadConfig(t *testing.T) { + configBytes := multicluster.DefaultControllerManagerConfigBytes + var multiclusterConfig mcsv1alpha1.MultiClusterConfig + o := newOptions() + assert.NoError(t, o.loadConfig(configBytes, &multiclusterConfig)) +} diff --git a/multicluster/embed.go b/multicluster/embed.go new file mode 100644 index 00000000000..e7bcb9e34a2 --- /dev/null +++ b/multicluster/embed.go @@ -0,0 +1,19 @@ +// Copyright 2024 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package multicluster + +import _ "embed" + +//go:embed config/default/configmap/controller_manager_config.yaml +var DefaultControllerManagerConfigBytes []byte