Skip to content

Commit

Permalink
kubeadm: implement mutators for "config migrate"
Browse files Browse the repository at this point in the history
When upconverting from v1beta3 to v1beta4, it appears there is no
easy way to migrate some of the timeout values such as:
  ClusterConfiguration.APIServer.TimeoutForControlPlane
to a new location:
  InitConfiguration.Timeouts.<some-timeout-field>

Yes, the internal InitConfiguratio does embed a ClusterConfiguration,
but during conversion the ClusterConfiguration is converted from an
empty source.

K8s' API machinery has ways to register custom conversion functions,
such v1beta3.ClusterConfiguration -> internal.InitConfiguration,
but these must be triggered explicitly with a decoder.
The overall migration of fields seems very awkward.

There might be hacks around that, such as storing intermediate state,
while trying to make the fuzzer rountrip happy, but instead
mutation functions can be implemented for the internal types when
calling kubeadm's migrate code. This seems much cleaner.
  • Loading branch information
neolit123 committed Dec 30, 2023
1 parent 781ba9b commit 601d0ad
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 3 deletions.
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func newCmdConfigMigrate(out io.Writer) *cobra.Command {
return err
}

outputBytes, err := configutil.MigrateOldConfig(oldCfgBytes, allowExperimental)
outputBytes, err := configutil.MigrateOldConfig(oldCfgBytes, allowExperimental, nil)
if err != nil {
return err
}
Expand Down
110 changes: 109 additions & 1 deletion cmd/kubeadm/app/util/config/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,13 @@ func validateKnownGVKs(gvks []schema.GroupVersionKind) error {

// MigrateOldConfig migrates an old configuration from a byte slice into a new one (returned again as a byte slice).
// Only kubeadm kinds are migrated.
func MigrateOldConfig(oldConfig []byte, allowExperimental bool) ([]byte, error) {
func MigrateOldConfig(oldConfig []byte, allowExperimental bool, mutators migrateMutators) ([]byte, error) {
newConfig := [][]byte{}

if mutators == nil {
mutators = defaultMigrateMutators()
}

gvkmap, err := kubeadmutil.SplitYAMLDocuments(oldConfig)
if err != nil {
return []byte{}, err
Expand All @@ -277,6 +281,9 @@ func MigrateOldConfig(oldConfig []byte, allowExperimental bool) ([]byte, error)
if err != nil {
return []byte{}, err
}
if err := mutators.mutate([]any{o}); err != nil {
return []byte{}, err
}
b, err := MarshalKubeadmConfigObject(o, gv)
if err != nil {
return []byte{}, err
Expand All @@ -290,6 +297,9 @@ func MigrateOldConfig(oldConfig []byte, allowExperimental bool) ([]byte, error)
if err != nil {
return []byte{}, err
}
if err := mutators.mutate([]any{o}); err != nil {
return []byte{}, err
}
b, err := MarshalKubeadmConfigObject(o, gv)
if err != nil {
return []byte{}, err
Expand All @@ -303,6 +313,9 @@ func MigrateOldConfig(oldConfig []byte, allowExperimental bool) ([]byte, error)
if err != nil {
return []byte{}, err
}
if err := mutators.mutate([]any{o}); err != nil {
return []byte{}, err
}
b, err := MarshalKubeadmConfigObject(o, gv)
if err != nil {
return []byte{}, err
Expand Down Expand Up @@ -383,3 +396,98 @@ func prepareStaticVariables(config interface{}) {
constants.SetAPICallTimeout(c.Timeouts.APICall.Duration)
}
}

// migrateMutator can be used to mutate a slice of configuration objects.
// The mutation is applied in-place and no copies are made.
type migrateMutator struct {
in []any
mutateFunc func(in []any) error
}

// migrateMutators holds a list of registered mutators.
type migrateMutators []migrateMutator

// mutate can be called on a list of registered mutators to find a suitable one to perform
// a configuration object mutation.
func (mutators migrateMutators) mutate(in []any) error {
var mutator *migrateMutator
for idx, m := range mutators {
if len(m.in) != len(in) {
continue
}
inputMatch := true
for idx := range m.in {
if reflect.TypeOf(m.in[idx]) != reflect.TypeOf(in[idx]) {
inputMatch = false
break
}
}
if inputMatch {
mutator = &mutators[idx]
break
}
}
if mutator == nil {
return errors.Errorf("could not find a mutator for input: %#v", in)
}
mutator.mutateFunc(in)
return nil
}

// addEmpty adds an empty migrate mutator for a given input.
func (mutators *migrateMutators) addEmpty(in []any) {
mutator := migrateMutator{
in: in,
mutateFunc: func(in []any) error { return nil },
}
*mutators = append(*mutators, mutator)
}

// defaultMutators returns the default list of mutators for known configuration objects.
// TODO: make this function return defaultEmptyMutators() when v1beta3 is removed.
func defaultMigrateMutators() migrateMutators {
var (
mutators migrateMutators
mutator migrateMutator
)

// mutator for InitConfiguration, ClusterConfiguration.
mutator = migrateMutator{
in: []any{(*kubeadmapi.InitConfiguration)(nil)},
mutateFunc: func(in []any) error {
a := in[0].(*kubeadmapi.InitConfiguration)
a.Timeouts.ControlPlaneComponentHealthCheck.Duration = a.APIServer.TimeoutForControlPlane.Duration
a.APIServer.TimeoutForControlPlane = nil
return nil
},
}
mutators = append(mutators, mutator)

// mutator for JoinConfiguration.
mutator = migrateMutator{
in: []any{(*kubeadmapi.JoinConfiguration)(nil)},
mutateFunc: func(in []any) error {
a := in[0].(*kubeadmapi.JoinConfiguration)
a.Timeouts.Discovery.Duration = a.Discovery.Timeout.Duration
a.Discovery.Timeout = nil
return nil
},
}
mutators = append(mutators, mutator)

// empty mutator for ResetConfiguration.
mutators.addEmpty([]any{(*kubeadmapi.ResetConfiguration)(nil)})

return mutators
}

// defaultEmptyMigrateMutators returns a list of empty mutators for known types.
func defaultEmptyMigrateMutators() migrateMutators {
mutators := &migrateMutators{}

mutators.addEmpty([]any{(*kubeadmapi.InitConfiguration)(nil)})
mutators.addEmpty([]any{(*kubeadmapi.JoinConfiguration)(nil)})
mutators.addEmpty([]any{(*kubeadmapi.ResetConfiguration)(nil)})

return *mutators
}
109 changes: 108 additions & 1 deletion cmd/kubeadm/app/util/config/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ import (
"fmt"
"reflect"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/lithammer/dedent"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/version"
apimachineryversion "k8s.io/apimachinery/pkg/version"
Expand Down Expand Up @@ -454,7 +458,7 @@ func TestMigrateOldConfig(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
b, err := MigrateOldConfig([]byte(test.oldCfg), test.allowExperimental)
b, err := MigrateOldConfig([]byte(test.oldCfg), test.allowExperimental, defaultEmptyMigrateMutators())
if test.expectErr {
if err == nil {
t.Fatalf("unexpected success:\n%s", b)
Expand Down Expand Up @@ -751,3 +755,106 @@ func TestNormalizeKubernetesVersion(t *testing.T) {
})
}
}

// TODO: update the test cases for this test once v1beta3 is removed.
func TestDefaultMigrateMutators(t *testing.T) {
tests := []struct {
name string
mutators migrateMutators
input []any
expected []any
expectedDiff bool
expectedError bool
}{
{
name: "mutate InitConfiguration",
mutators: defaultMigrateMutators(),
input: []any{&kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
APIServer: kubeadmapi.APIServer{
TimeoutForControlPlane: &metav1.Duration{
Duration: 1234 * time.Millisecond,
},
},
},
Timeouts: &kubeadmapi.Timeouts{
ControlPlaneComponentHealthCheck: &metav1.Duration{},
},
}},
expected: []any{&kubeadmapi.InitConfiguration{
Timeouts: &kubeadmapi.Timeouts{
ControlPlaneComponentHealthCheck: &metav1.Duration{
Duration: 1234 * time.Millisecond,
},
},
}},
},
{
name: "mutate JoinConfiguration",
mutators: defaultMigrateMutators(),
input: []any{&kubeadmapi.JoinConfiguration{
Discovery: kubeadmapi.Discovery{
Timeout: &metav1.Duration{
Duration: 1234 * time.Microsecond,
},
},
Timeouts: &kubeadmapi.Timeouts{
Discovery: &metav1.Duration{},
},
}},
expected: []any{&kubeadmapi.JoinConfiguration{
Timeouts: &kubeadmapi.Timeouts{
Discovery: &metav1.Duration{
Duration: 1234 * time.Microsecond,
},
},
}},
},
{
name: "diff when mutating InitConfiguration",
mutators: defaultMigrateMutators(),
input: []any{&kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
APIServer: kubeadmapi.APIServer{
TimeoutForControlPlane: &metav1.Duration{
Duration: 1234 * time.Millisecond,
},
},
},
Timeouts: &kubeadmapi.Timeouts{
ControlPlaneComponentHealthCheck: &metav1.Duration{},
},
}},
expected: []any{&kubeadmapi.InitConfiguration{
Timeouts: &kubeadmapi.Timeouts{
ControlPlaneComponentHealthCheck: &metav1.Duration{
Duration: 1 * time.Millisecond, // a different value
},
},
}},
expectedDiff: true,
},
{
name: "expect an error for a missing mutator",
mutators: migrateMutators{}, // empty list of mutators
input: []any{&kubeadmapi.ResetConfiguration{}},
expectedError: true,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := tc.mutators.mutate(tc.input)
if (err != nil) != tc.expectedError {
t.Fatalf("expected error: %v, got: %v, error: %v", tc.expectedError, (err != nil), err)
}
if err != nil {
return
}
diff := cmp.Diff(tc.expected, tc.input)
if (len(diff) > 0) != tc.expectedDiff {
t.Fatalf("got a diff with the expected config (-want,+got):\n%s", diff)
}
})
}
}

0 comments on commit 601d0ad

Please sign in to comment.