Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kubeadm: config migrate handles more valid configs #71315

Merged
merged 1 commit into from
Nov 22, 2018
Merged

kubeadm: config migrate handles more valid configs #71315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions cmd/kubeadm/app/cmd/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,10 +329,7 @@ func NewCmdConfigMigrate(out io.Writer) *cobra.Command {
kubeadmutil.CheckErr(errors.New("The --old-config flag is mandatory"))
}

internalcfg, err := configutil.AnyConfigFileAndDefaultsToInternal(oldCfgPath)
kubeadmutil.CheckErr(err)

outputBytes, err := configutil.MarshalKubeadmConfigObject(internalcfg)
outputBytes, err := configutil.MigrateOldConfigFromFile(oldCfgPath)
kubeadmutil.CheckErr(err)

if newCfgPath == "" {
Expand Down
1 change: 1 addition & 0 deletions cmd/kubeadm/app/util/config/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ go_test(
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
"//vendor/github.com/pmezard/go-difflib/difflib:go_default_library",
"//vendor/github.com/renstrom/dedent:go_default_library",
],
)

Expand Down
66 changes: 44 additions & 22 deletions cmd/kubeadm/app/util/config/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package config

import (
"bytes"
"io/ioutil"
"net"
"reflect"
Expand All @@ -35,28 +36,6 @@ import (
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
)

// AnyConfigFileAndDefaultsToInternal reads either a InitConfiguration or JoinConfiguration and unmarshals it
func AnyConfigFileAndDefaultsToInternal(cfgPath string) (runtime.Object, error) {
b, err := ioutil.ReadFile(cfgPath)
if err != nil {
return nil, err
}

gvks, err := kubeadmutil.GroupVersionKindsFromBytes(b)
if err != nil {
return nil, err
}

// First, check if the gvk list has InitConfiguration and in that case try to unmarshal it
if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvks...) {
return ConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.InitConfiguration{})
}
if kubeadmutil.GroupVersionKindsHasJoinConfiguration(gvks...) {
return JoinConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.JoinConfiguration{})
}
return nil, errors.Errorf("didn't recognize types with GroupVersionKind: %v", gvks)
}

// MarshalKubeadmConfigObject marshals an Object registered in the kubeadm scheme. If the object is a InitConfiguration or ClusterConfiguration, some extra logic is run
func MarshalKubeadmConfigObject(obj runtime.Object) ([]byte, error) {
switch internalcfg := obj.(type) {
Expand Down Expand Up @@ -181,3 +160,46 @@ func ChooseAPIServerBindAddress(bindAddress net.IP) (net.IP, error) {
}
return ip, nil
}

// MigrateOldConfigFromFile migrates an old configuration from a file into a new one (returned as a byte slice). Only kubeadm kinds are migrated. Others are silently ignored.
func MigrateOldConfigFromFile(cfgPath string) ([]byte, error) {
newConfig := [][]byte{}

cfgBytes, err := ioutil.ReadFile(cfgPath)
if err != nil {
return []byte{}, err
}

gvks, err := kubeadmutil.GroupVersionKindsFromBytes(cfgBytes)
if err != nil {
return []byte{}, err
}

// Migrate InitConfiguration and ClusterConfiguration if there are any in the config
if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvks...) || kubeadmutil.GroupVersionKindsHasClusterConfiguration(gvks...) {
o, err := ConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.InitConfiguration{})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do some refactoring for ConfigFileAndDefaultsToInternalConfig, since the file will be read twice. There is no need.

Copy link
Contributor Author

@rosti rosti Nov 21, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's on my agenda for the next release cycle. Decoupling InitConfiguration and ClusterConfiguration is one of my priorities. If we refactor the APIs and make ClusterConfiguration more standalone and introduce an internal type that houses the usual pair (ClusterConfiguration + NodeRegistrationOptions for the current node) we can eliminate most of the confusion and optimize things at all levels.
But for now at the end of this release cycle it's best to keep the changes as small as possible and just fix the bug at hand.

if err != nil {
return []byte{}, err
}
b, err := MarshalKubeadmConfigObject(o)
if err != nil {
return []byte{}, err
}
newConfig = append(newConfig, b)
}

// Migrate JoinConfiguration if there is any
if kubeadmutil.GroupVersionKindsHasJoinConfiguration(gvks...) {
o, err := JoinConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.JoinConfiguration{})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

if err != nil {
return []byte{}, err
}
b, err := MarshalKubeadmConfigObject(o)
if err != nil {
return []byte{}, err
}
newConfig = append(newConfig, b)
}

return bytes.Join(newConfig, []byte(constants.YAMLDocumentSeparator)), nil
}
214 changes: 214 additions & 0 deletions cmd/kubeadm/app/util/config/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@ package config

import (
"bytes"
"io/ioutil"
"os"
"testing"

"github.com/renstrom/dedent"

kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1"
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
)

var files = map[string][]byte{
Expand Down Expand Up @@ -298,3 +303,212 @@ func TestVerifyAPIServerBindAddress(t *testing.T) {
})
}
}

func TestMigrateOldConfigFromFile(t *testing.T) {
tests := []struct {
desc string
oldCfg string
expectedKinds []string
expectErr bool
}{
{
desc: "empty file produces empty result",
oldCfg: "",
expectErr: false,
},
{
desc: "bad config produces error",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
`),
expectErr: true,
},
{
desc: "InitConfiguration only gets migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: InitConfiguration
`),
expectedKinds: []string{
constants.InitConfigurationKind,
constants.ClusterConfigurationKind,
},
expectErr: false,
},
{
desc: "ClusterConfiguration only gets migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: ClusterConfiguration
`),
expectedKinds: []string{
constants.InitConfigurationKind,
constants.ClusterConfigurationKind,
},
expectErr: false,
},
{
desc: "JoinConfiguration only gets migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: JoinConfiguration
token: abcdef.0123456789abcdef
discoveryTokenAPIServers:
- kube-apiserver:6443
discoveryTokenUnsafeSkipCAVerification: true
`),
expectedKinds: []string{
constants.JoinConfigurationKind,
},
expectErr: false,
},
{
desc: "Init + Cluster Configurations are migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: InitConfiguration
---
apiVersion: kubeadm.k8s.io/v1alpha3
kind: ClusterConfiguration
`),
expectedKinds: []string{
constants.InitConfigurationKind,
constants.ClusterConfigurationKind,
},
expectErr: false,
},
{
desc: "Init + Join Configurations are migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: InitConfiguration
---
apiVersion: kubeadm.k8s.io/v1alpha3
kind: JoinConfiguration
token: abcdef.0123456789abcdef
discoveryTokenAPIServers:
- kube-apiserver:6443
discoveryTokenUnsafeSkipCAVerification: true
`),
expectedKinds: []string{
constants.InitConfigurationKind,
constants.ClusterConfigurationKind,
constants.JoinConfigurationKind,
},
expectErr: false,
},
{
desc: "Cluster + Join Configurations are migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: ClusterConfiguration
---
apiVersion: kubeadm.k8s.io/v1alpha3
kind: JoinConfiguration
token: abcdef.0123456789abcdef
discoveryTokenAPIServers:
- kube-apiserver:6443
discoveryTokenUnsafeSkipCAVerification: true
`),
expectedKinds: []string{
constants.InitConfigurationKind,
constants.ClusterConfigurationKind,
constants.JoinConfigurationKind,
},
expectErr: false,
},
{
desc: "Init + Cluster + Join Configurations are migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: InitConfiguration
---
apiVersion: kubeadm.k8s.io/v1alpha3
kind: ClusterConfiguration
---
apiVersion: kubeadm.k8s.io/v1alpha3
kind: JoinConfiguration
token: abcdef.0123456789abcdef
discoveryTokenAPIServers:
- kube-apiserver:6443
discoveryTokenUnsafeSkipCAVerification: true
`),
expectedKinds: []string{
constants.InitConfigurationKind,
constants.ClusterConfigurationKind,
constants.JoinConfigurationKind,
},
expectErr: false,
},
{
desc: "component configs are not migrated",
oldCfg: dedent.Dedent(`
apiVersion: kubeadm.k8s.io/v1alpha3
kind: InitConfiguration
---
apiVersion: kubeadm.k8s.io/v1alpha3
kind: ClusterConfiguration
---
apiVersion: kubeadm.k8s.io/v1alpha3
kind: JoinConfiguration
token: abcdef.0123456789abcdef
discoveryTokenAPIServers:
- kube-apiserver:6443
discoveryTokenUnsafeSkipCAVerification: true
---
apiVersion: kubeproxy.config.k8s.io/v1alpha1
kind: KubeProxyConfiguration
---
apiVersion: kubelet.config.k8s.io/v1beta1
kind: KubeletConfiguration
`),
expectedKinds: []string{
constants.InitConfigurationKind,
constants.ClusterConfigurationKind,
constants.JoinConfigurationKind,
},
expectErr: false,
},
}

for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
file, err := ioutil.TempFile("", "")
if err != nil {
t.Fatalf("could not create temporary test file: %v", err)
}
fileName := file.Name()
defer os.Remove(fileName)

_, err = file.WriteString(test.oldCfg)
file.Close()
if err != nil {
t.Fatalf("could not write contents of old config: %v", err)
}

b, err := MigrateOldConfigFromFile(fileName)
if test.expectErr {
if err == nil {
t.Fatalf("unexpected success:\n%s", b)
}
} else {
if err != nil {
t.Fatalf("unexpected failure: %v", err)
}
gvks, err := kubeadmutil.GroupVersionKindsFromBytes(b)
if err != nil {
t.Fatalf("unexpected error returned by GroupVersionKindsFromBytes: %v", err)
}
if len(gvks) != len(test.expectedKinds) {
t.Fatalf("length mismatch between resulting gvks and expected kinds:\n\tlen(gvks)=%d\n\tlen(expectedKinds)=%d",
len(gvks), len(test.expectedKinds))
}
for _, expectedKind := range test.expectedKinds {
if !kubeadmutil.GroupVersionKindsHasKind(gvks, expectedKind) {
t.Fatalf("migration failed to produce config kind: %s", expectedKind)
}
}
}
})
}
}