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.

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

kubeadm: UnifiedControlPlaneImage string -> UseHyperKubeImage bool #70793

Merged
merged 1 commit into from
Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 3 deletions cmd/kubeadm/app/apis/kubeadm/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@ type ClusterConfiguration struct {
// +k8s:conversion-gen=false
CIImageRepository string

// UnifiedControlPlaneImage specifies if a specific container image should be
// used for all control plane components.
UnifiedControlPlaneImage string
// UseHyperKubeImage controls if hyperkube should be used for Kubernetes components instead of their respective separate images
UseHyperKubeImage bool
Copy link
Member

Choose a reason for hiding this comment

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

someone might decide to build a custom "unified control plane" image that is not called hyperkube.
in such a case they would have to name it "hyperkube" for kubeadm to be able to work with it.

this at least solves upgrade issues for us...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If some users override this with their own image, then it should be based on hyperkube. Hence tagging it appropriately and setting it up on their own repository is the way to go.
Definitely we should not allow users to put whatever image they see fit in here and the bool value is the best solution for me.


// AuditPolicyConfiguration defines the options for the api server audit system.
AuditPolicyConfiguration AuditPolicyConfiguration
Expand Down
2 changes: 2 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/v1alpha3/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ go_library(
deps = [
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
"//cmd/kubeadm/app/constants:go_default_library",
"//cmd/kubeadm/app/images:go_default_library",
"//cmd/kubeadm/app/util:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library",
Expand Down
29 changes: 29 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/v1alpha3/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ package v1alpha3

import (
"github.com/pkg/errors"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/images"
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
)

func Convert_v1alpha3_JoinConfiguration_To_kubeadm_JoinConfiguration(in *JoinConfiguration, out *kubeadm.JoinConfiguration, s conversion.Scope) error {
Expand Down Expand Up @@ -108,9 +111,29 @@ func Convert_v1alpha3_ClusterConfiguration_To_kubeadm_ClusterConfiguration(in *C
return err
}

if err := Convert_v1alpha3_UnifiedControlPlaneImage_To_kubeadm_UseHyperKubeImage(in, out); err != nil {
return err
}

return nil
}

func Convert_v1alpha3_UnifiedControlPlaneImage_To_kubeadm_UseHyperKubeImage(in *ClusterConfiguration, out *kubeadm.ClusterConfiguration) error {
if len(in.UnifiedControlPlaneImage) == 0 {
out.UseHyperKubeImage = false
return nil
}

k8sImageTag := kubeadmutil.KubernetesVersionToImageTag(in.KubernetesVersion)
expectedImage := images.GetGenericImage(in.ImageRepository, constants.HyperKube, k8sImageTag)
if expectedImage == in.UnifiedControlPlaneImage {
out.UseHyperKubeImage = true
return nil
}

return errors.Errorf("cannot convert unifiedControlPlaneImage=%q to useHyperKubeImage", in.UnifiedControlPlaneImage)
}

func Convert_kubeadm_ClusterConfiguration_To_v1alpha3_ClusterConfiguration(in *kubeadm.ClusterConfiguration, out *ClusterConfiguration, s conversion.Scope) error {
if err := autoConvert_kubeadm_ClusterConfiguration_To_v1alpha3_ClusterConfiguration(in, out, s); err != nil {
return err
Expand All @@ -132,6 +155,12 @@ func Convert_kubeadm_ClusterConfiguration_To_v1alpha3_ClusterConfiguration(in *k
return err
}

if in.UseHyperKubeImage {
out.UnifiedControlPlaneImage = images.GetKubeControlPlaneImage("", in)
} else {
out.UnifiedControlPlaneImage = ""
}

return nil
}

Expand Down
72 changes: 72 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/v1alpha3/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,75 @@ func TestJoinConfigurationConversion(t *testing.T) {
}
}
}

func TestConvertToUseHyperKubeImage(t *testing.T) {
tests := []struct {
desc string
in *v1alpha3.ClusterConfiguration
useHyperKubeImage bool
expectedErr bool
}{
{
desc: "unset UnifiedControlPlaneImage sets UseHyperKubeImage to false",
in: &v1alpha3.ClusterConfiguration{},
useHyperKubeImage: false,
expectedErr: false,
},
{
desc: "matching UnifiedControlPlaneImage sets UseHyperKubeImage to true",
in: &v1alpha3.ClusterConfiguration{
ImageRepository: "k8s.gcr.io",
KubernetesVersion: "v1.12.2",
UnifiedControlPlaneImage: "k8s.gcr.io/hyperkube:v1.12.2",
},
useHyperKubeImage: true,
expectedErr: false,
},
{
desc: "mismatching UnifiedControlPlaneImage tag causes an error",
in: &v1alpha3.ClusterConfiguration{
ImageRepository: "k8s.gcr.io",
KubernetesVersion: "v1.12.0",
UnifiedControlPlaneImage: "k8s.gcr.io/hyperkube:v1.12.2",
},
expectedErr: true,
},
{
desc: "mismatching UnifiedControlPlaneImage repo causes an error",
in: &v1alpha3.ClusterConfiguration{
ImageRepository: "my.repo",
KubernetesVersion: "v1.12.2",
UnifiedControlPlaneImage: "k8s.gcr.io/hyperkube:v1.12.2",
},
expectedErr: true,
},
{
desc: "mismatching UnifiedControlPlaneImage image name causes an error",
in: &v1alpha3.ClusterConfiguration{
ImageRepository: "k8s.gcr.io",
KubernetesVersion: "v1.12.2",
UnifiedControlPlaneImage: "k8s.gcr.io/otherimage:v1.12.2",
},
expectedErr: true,
},
}

for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
out := &kubeadm.ClusterConfiguration{}
err := v1alpha3.Convert_v1alpha3_UnifiedControlPlaneImage_To_kubeadm_UseHyperKubeImage(test.in, out)
if test.expectedErr {
if err == nil {
t.Fatalf("unexpected success, UseHyperKubeImage: %t", out.UseHyperKubeImage)
}
} else {
if err != nil {
t.Fatalf("unexpected failure: %v", err)
}
if out.UseHyperKubeImage != test.useHyperKubeImage {
t.Fatalf("mismatching result from conversion:\n\tExpected: %t\n\tReceived: %t", test.useHyperKubeImage, out.UseHyperKubeImage)
}
}
})
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cmd/kubeadm/app/apis/kubeadm/v1beta1/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ limitations under the License.
// pathType: File
// certificatesDir: "/etc/kubernetes/pki"
// imageRepository: "k8s.gcr.io"
// unifiedControlPlaneImage: "k8s.gcr.io/controlplane:v1.12.0"
// useHyperKubeImage: false
// auditPolicy:
// # https://kubernetes.io/docs/tasks/debug-application-cluster/audit/#audit-policy
// path: "/var/log/audit/audit.json"
Expand Down
6 changes: 3 additions & 3 deletions cmd/kubeadm/app/apis/kubeadm/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ type ClusterConfiguration struct {

// ImageRepository what container registry to pull control plane images from
ImageRepository string `json:"imageRepository"`
// UnifiedControlPlaneImage specifies if a specific container image should
// be used for all control plane components.
UnifiedControlPlaneImage string `json:"unifiedControlPlaneImage"`

// UseHyperKubeImage controls if hyperkube should be used for Kubernetes components instead of their respective separate images
UseHyperKubeImage bool `json:"useHyperKubeImage,omitempty"`

// AuditPolicyConfiguration defines the options for the api server audit system
AuditPolicyConfiguration AuditPolicyConfiguration `json:"auditPolicy"`
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions cmd/kubeadm/app/cmd/upgrade/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func TestPrintConfiguration(t *testing.T) {
podSubnet: ""
serviceSubnet: ""
scheduler: {}
unifiedControlPlaneImage: ""
`),
},
{
Expand Down Expand Up @@ -102,7 +101,6 @@ func TestPrintConfiguration(t *testing.T) {
podSubnet: ""
serviceSubnet: 10.96.0.1/12
scheduler: {}
unifiedControlPlaneImage: ""
`),
},
}
Expand Down
2 changes: 2 additions & 0 deletions cmd/kubeadm/app/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ const (
KubeScheduler = "kube-scheduler"
// KubeProxy defines variable used internally when referring to kube-proxy component
KubeProxy = "kube-proxy"
// HyperKube defines variable used internally when referring to the hyperkube image
HyperKube = "hyperkube"

// SelfHostingPrefix describes the prefix workloads that are self-hosted by kubeadm has
SelfHostingPrefix = "self-hosted-"
Expand Down
18 changes: 12 additions & 6 deletions cmd/kubeadm/app/images/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ func GetGenericImage(prefix, image, tag string) string {

// GetKubeControlPlaneImage generates and returns the image for the core Kubernetes components or returns the unified control plane image if specified
func GetKubeControlPlaneImage(image string, cfg *kubeadmapi.ClusterConfiguration) string {
if cfg.UnifiedControlPlaneImage != "" {
return cfg.UnifiedControlPlaneImage
if cfg.UseHyperKubeImage {
Copy link
Member

Choose a reason for hiding this comment

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

If you change the line below I don't think this is necessary

image = constants.HyperKube
}
repoPrefix := cfg.GetControlPlaneImageRepository()
kubernetesImageTag := kubeadmutil.KubernetesVersionToImageTag(cfg.KubernetesVersion)
Expand All @@ -56,10 +56,16 @@ func GetEtcdImage(cfg *kubeadmapi.ClusterConfiguration) string {
// GetAllImages returns a list of container images kubeadm expects to use on a control plane node
func GetAllImages(cfg *kubeadmapi.ClusterConfiguration) []string {
imgs := []string{}
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeAPIServer, cfg))
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeControllerManager, cfg))
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeScheduler, cfg))
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeProxy, cfg))

// start with core kubernetes images
if cfg.UseHyperKubeImage {
imgs = append(imgs, GetKubeControlPlaneImage(constants.HyperKube, cfg))
} else {
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeAPIServer, cfg))
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeControllerManager, cfg))
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeScheduler, cfg))
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeProxy, cfg))
}

// pause, etcd and kube-dns are not available on the ci image repository so use the default image repository.
imgs = append(imgs, GetGenericImage(cfg.ImageRepository, "pause", constants.PauseVersion))
Expand Down
6 changes: 4 additions & 2 deletions cmd/kubeadm/app/images/images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ func TestGetKubeControlPlaneImage(t *testing.T) {
cfg *kubeadmapi.ClusterConfiguration
}{
{
expected: "override",
expected: GetGenericImage(gcrPrefix, constants.HyperKube, expected),
cfg: &kubeadmapi.ClusterConfiguration{
UnifiedControlPlaneImage: "override",
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
UseHyperKubeImage: true,
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/phases/upgrade/staticpods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ networking:
schedulerExtraArgs: null
token: ce3aa5.5ec8455bb76b379f
tokenTTL: 24h
unifiedControlPlaneImage: ""
useHyperKubeImage: false
`
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,4 @@ NodeRegistration:
Scheduler:
ExtraArgs: null
ExtraVolumes: null
UnifiedControlPlaneImage: ""
UseHyperKubeImage: true
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ networking:
dnsDomain: cluster.local
podSubnet: ""
serviceSubnet: 10.96.0.0/12
unifiedControlPlaneImage: ""
unifiedControlPlaneImage: "k8s.gcr.io/hyperkube:v1.11.2"
---
apiVersion: kubeproxy.config.k8s.io/v1alpha1
bindAddress: 0.0.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ networking:
podSubnet: ""
serviceSubnet: 10.96.0.0/12
scheduler: {}
unifiedControlPlaneImage: ""
useHyperKubeImage: true
---
apiVersion: kubeproxy.config.k8s.io/v1alpha1
bindAddress: 0.0.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ networking:
podSubnet: 10.148.0.0/16
serviceSubnet: 10.196.0.0/12
scheduler: {}
unifiedControlPlaneImage: ""
---
apiVersion: kubeproxy.config.k8s.io/v1alpha1
bindAddress: 0.0.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ networking:
dnsDomain: INVALID-DOMAIN-!!!!
podSubnet: ""
serviceSubnet: 10.96.0.0/12
unifiedControlPlaneImage: ""
useHyperKubeImage: false