Skip to content

Commit

Permalink
Avoid error log when unmarshalling config for MC controller (antrea-i…
Browse files Browse the repository at this point in the history
…o#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 <antonin.bas@broadcom.com>
  • Loading branch information
antoninbas authored and hangyan committed Oct 29, 2024
1 parent 8b006ab commit 5b65660
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 12 deletions.
22 changes: 10 additions & 12 deletions multicluster/cmd/multicluster-controller/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
20 changes: 20 additions & 0 deletions multicluster/cmd/multicluster-controller/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
}
19 changes: 19 additions & 0 deletions multicluster/embed.go
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 5b65660

Please sign in to comment.