Skip to content

Commit

Permalink
Merge pull request kubernetes#67503 from rosti/kubeadm_clusterconfig_…
Browse files Browse the repository at this point in the history
…images

Automatic merge from submit-queue (batch tested with PRs 67776, 67503, 67679, 67786, 67830). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

 kubeadm: use ClusterConfiguration in images.go

**What this PR does / why we need it**:

This PR is the first in a series, targeting the replacement of InitConfiguration with ClusterConfiguration, when the former is not needed. Please, review only the last commit.

Replace the unnecessary use of InitConfiguration in images.go with ClusterConfiguration. This changes the interfaces of the following functions:

- GetKubeControlPlaneImage
- GetEtcdImage
- GetAllImages

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
refs kubernetes/kubeadm#963

**Special notes for your reviewer**:

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/kind enhancement
/assign @luxas
/assign @timothysc
/assign @fabriziopandini

Depends on:
- [X] kubernetes#67441

**Release note**:

```release-note
NONE
```
  • Loading branch information
Kubernetes Submit Queue committed Aug 24, 2018
2 parents a11dcec + de39f49 commit de80c82
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 81 deletions.
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/apis/kubeadm/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ type JoinConfiguration struct {
// It will override location with CI registry name in case user requests special
// Kubernetes version from CI build area.
// (See: kubeadmconstants.DefaultCIImageRepository)
func (cfg *InitConfiguration) GetControlPlaneImageRepository() string {
func (cfg *ClusterConfiguration) GetControlPlaneImageRepository() string {
if cfg.CIImageRepository != "" {
return cfg.CIImageRepository
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/kubeadm/app/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ func NewCmdConfigImagesPull() *cobra.Command {
kubeadmutil.CheckErr(err)
containerRuntime, err := utilruntime.NewContainerRuntime(utilsexec.New(), internalcfg.GetCRISocket())
kubeadmutil.CheckErr(err)
imagesPull := NewImagesPull(containerRuntime, images.GetAllImages(internalcfg))
imagesPull := NewImagesPull(containerRuntime, images.GetAllImages(&internalcfg.ClusterConfiguration))
kubeadmutil.CheckErr(imagesPull.PullAll())
},
}
Expand Down Expand Up @@ -516,7 +516,7 @@ type ImagesList struct {

// Run runs the images command and writes the result to the io.Writer passed in
func (i *ImagesList) Run(out io.Writer) error {
imgs := images.GetAllImages(i.cfg)
imgs := images.GetAllImages(&i.cfg.ClusterConfiguration)
for _, img := range imgs {
fmt.Fprintln(out, img)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/cmd/upgrade/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func RunApply(flags *applyFlags) error {
// Use a prepuller implementation based on creating DaemonSets
// and block until all DaemonSets are ready; then we know for sure that all control plane images are cached locally
glog.V(1).Infof("[upgrade/apply] creating prepuller")
prepuller := upgrade.NewDaemonSetPrepuller(upgradeVars.client, upgradeVars.waiter, upgradeVars.cfg)
prepuller := upgrade.NewDaemonSetPrepuller(upgradeVars.client, upgradeVars.waiter, &upgradeVars.cfg.ClusterConfiguration)
if err := upgrade.PrepullImagesInParallel(prepuller, flags.imagePullTimeout); err != nil {
return fmt.Errorf("[upgrade/prepull] Failed prepulled the images for the control plane components error: %v", err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/kubeadm/app/images/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func GetGenericArchImage(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.InitConfiguration) string {
func GetKubeControlPlaneImage(image string, cfg *kubeadmapi.ClusterConfiguration) string {
if cfg.UnifiedControlPlaneImage != "" {
return cfg.UnifiedControlPlaneImage
}
Expand All @@ -47,7 +47,7 @@ func GetKubeControlPlaneImage(image string, cfg *kubeadmapi.InitConfiguration) s
}

// GetEtcdImage generates and returns the image for etcd or returns cfg.Etcd.Local.Image if specified
func GetEtcdImage(cfg *kubeadmapi.InitConfiguration) string {
func GetEtcdImage(cfg *kubeadmapi.ClusterConfiguration) string {
if cfg.Etcd.Local != nil && cfg.Etcd.Local.Image != "" {
return cfg.Etcd.Local.Image
}
Expand All @@ -60,7 +60,7 @@ func GetEtcdImage(cfg *kubeadmapi.InitConfiguration) string {
}

// GetAllImages returns a list of container images kubeadm expects to use on a control plane node
func GetAllImages(cfg *kubeadmapi.InitConfiguration) []string {
func GetAllImages(cfg *kubeadmapi.ClusterConfiguration) []string {
imgs := []string{}
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeAPIServer, cfg))
imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeControllerManager, cfg))
Expand Down
106 changes: 40 additions & 66 deletions cmd/kubeadm/app/images/images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,44 +49,36 @@ func TestGetKubeControlPlaneImage(t *testing.T) {
var tests = []struct {
image string
expected string
cfg *kubeadmapi.InitConfiguration
cfg *kubeadmapi.ClusterConfiguration
}{
{
expected: "override",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
UnifiedControlPlaneImage: "override",
},
cfg: &kubeadmapi.ClusterConfiguration{
UnifiedControlPlaneImage: "override",
},
},
{
image: constants.KubeAPIServer,
expected: GetGenericArchImage(gcrPrefix, "kube-apiserver", expected),
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
cfg: &kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
},
{
image: constants.KubeControllerManager,
expected: GetGenericArchImage(gcrPrefix, "kube-controller-manager", expected),
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
cfg: &kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
},
{
image: constants.KubeScheduler,
expected: GetGenericArchImage(gcrPrefix, "kube-scheduler", expected),
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
cfg: &kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
},
}
Expand All @@ -105,27 +97,23 @@ func TestGetKubeControlPlaneImage(t *testing.T) {
func TestGetEtcdImage(t *testing.T) {
var tests = []struct {
expected string
cfg *kubeadmapi.InitConfiguration
cfg *kubeadmapi.ClusterConfiguration
}{
{
expected: "override",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
Etcd: kubeadmapi.Etcd{
Local: &kubeadmapi.LocalEtcd{
Image: "override",
},
cfg: &kubeadmapi.ClusterConfiguration{
Etcd: kubeadmapi.Etcd{
Local: &kubeadmapi.LocalEtcd{
Image: "override",
},
},
},
},
{
expected: GetGenericArchImage(gcrPrefix, "etcd", constants.DefaultEtcdVersion),
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
cfg: &kubeadmapi.ClusterConfiguration{
ImageRepository: gcrPrefix,
KubernetesVersion: testversion,
},
},
}
Expand All @@ -144,78 +132,64 @@ func TestGetEtcdImage(t *testing.T) {
func TestGetAllImages(t *testing.T) {
testcases := []struct {
name string
cfg *kubeadmapi.InitConfiguration
expect string
cfg *kubeadmapi.ClusterConfiguration
}{
{
name: "defined CIImageRepository",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
CIImageRepository: "test.repo",
},
cfg: &kubeadmapi.ClusterConfiguration{
CIImageRepository: "test.repo",
},
expect: "test.repo",
},
{
name: "undefined CIImagerRepository should contain the default image prefix",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
ImageRepository: "real.repo",
},
cfg: &kubeadmapi.ClusterConfiguration{
ImageRepository: "real.repo",
},
expect: "real.repo",
},
{
name: "test that etcd is returned when it is not external",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
Etcd: kubeadmapi.Etcd{
Local: &kubeadmapi.LocalEtcd{},
},
cfg: &kubeadmapi.ClusterConfiguration{
Etcd: kubeadmapi.Etcd{
Local: &kubeadmapi.LocalEtcd{},
},
},
expect: constants.Etcd,
},
{
name: "CoreDNS image is returned",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": true,
},
cfg: &kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": true,
},
},
expect: constants.CoreDNS,
},
{
name: "main kube-dns image is returned",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": false,
},
cfg: &kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": false,
},
},
expect: "k8s-dns-kube-dns",
},
{
name: "kube-dns sidecar image is returned",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": false,
},
cfg: &kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": false,
},
},
expect: "k8s-dns-sidecar",
},
{
name: "kube-dns dnsmasq-nanny image is returned",
cfg: &kubeadmapi.InitConfiguration{
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": false,
},
cfg: &kubeadmapi.ClusterConfiguration{
FeatureGates: map[string]bool{
"CoreDNS": false,
},
},
expect: "k8s-dns-dnsmasq-nanny",
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/phases/addons/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func EnsureProxyAddon(cfg *kubeadmapi.InitConfiguration, client clientset.Interf
return fmt.Errorf("error when parsing kube-proxy configmap template: %v", err)
}
proxyDaemonSetBytes, err = kubeadmutil.ParseTemplate(KubeProxyDaemonSet19, struct{ Image, Arch string }{
Image: images.GetKubeControlPlaneImage(constants.KubeProxy, cfg),
Image: images.GetKubeControlPlaneImage(constants.KubeProxy, &cfg.ClusterConfiguration),
Arch: runtime.GOARCH,
})
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions cmd/kubeadm/app/phases/controlplane/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.InitConfiguration, k8sVersion *version.Ve
staticPodSpecs := map[string]v1.Pod{
kubeadmconstants.KubeAPIServer: staticpodutil.ComponentPod(v1.Container{
Name: kubeadmconstants.KubeAPIServer,
Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeAPIServer, cfg),
Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeAPIServer, &cfg.ClusterConfiguration),
ImagePullPolicy: v1.PullIfNotPresent,
Command: getAPIServerCommand(cfg),
VolumeMounts: staticpodutil.VolumeMountMapToSlice(mounts.GetVolumeMounts(kubeadmconstants.KubeAPIServer)),
Expand All @@ -83,7 +83,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.InitConfiguration, k8sVersion *version.Ve
}, mounts.GetVolumes(kubeadmconstants.KubeAPIServer)),
kubeadmconstants.KubeControllerManager: staticpodutil.ComponentPod(v1.Container{
Name: kubeadmconstants.KubeControllerManager,
Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeControllerManager, cfg),
Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeControllerManager, &cfg.ClusterConfiguration),
ImagePullPolicy: v1.PullIfNotPresent,
Command: getControllerManagerCommand(cfg, k8sVersion),
VolumeMounts: staticpodutil.VolumeMountMapToSlice(mounts.GetVolumeMounts(kubeadmconstants.KubeControllerManager)),
Expand All @@ -93,7 +93,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.InitConfiguration, k8sVersion *version.Ve
}, mounts.GetVolumes(kubeadmconstants.KubeControllerManager)),
kubeadmconstants.KubeScheduler: staticpodutil.ComponentPod(v1.Container{
Name: kubeadmconstants.KubeScheduler,
Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeScheduler, cfg),
Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeScheduler, &cfg.ClusterConfiguration),
ImagePullPolicy: v1.PullIfNotPresent,
Command: getSchedulerCommand(cfg),
VolumeMounts: staticpodutil.VolumeMountMapToSlice(mounts.GetVolumeMounts(kubeadmconstants.KubeScheduler)),
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/phases/etcd/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func GetEtcdPodSpec(cfg *kubeadmapi.InitConfiguration) v1.Pod {
return staticpodutil.ComponentPod(v1.Container{
Name: kubeadmconstants.Etcd,
Command: getEtcdCommand(cfg),
Image: images.GetEtcdImage(cfg),
Image: images.GetEtcdImage(&cfg.ClusterConfiguration),
ImagePullPolicy: v1.PullIfNotPresent,
// Mount the etcd datadir path read-write so etcd can store data in a more persistent manner
VolumeMounts: []v1.VolumeMount{
Expand Down
4 changes: 2 additions & 2 deletions cmd/kubeadm/app/phases/upgrade/prepull.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ type Prepuller interface {
// DaemonSetPrepuller makes sure the control plane images are available on all masters
type DaemonSetPrepuller struct {
client clientset.Interface
cfg *kubeadmapi.InitConfiguration
cfg *kubeadmapi.ClusterConfiguration
waiter apiclient.Waiter
}

// NewDaemonSetPrepuller creates a new instance of the DaemonSetPrepuller struct
func NewDaemonSetPrepuller(client clientset.Interface, waiter apiclient.Waiter, cfg *kubeadmapi.InitConfiguration) *DaemonSetPrepuller {
func NewDaemonSetPrepuller(client clientset.Interface, waiter apiclient.Waiter, cfg *kubeadmapi.ClusterConfiguration) *DaemonSetPrepuller {
return &DaemonSetPrepuller{
client: client,
cfg: cfg,
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/preflight/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ func RunPullImagesCheck(execer utilsexec.Interface, cfg *kubeadmapi.InitConfigur
}

checks := []Checker{
ImagePullCheck{runtime: containerRuntime, imageList: images.GetAllImages(cfg)},
ImagePullCheck{runtime: containerRuntime, imageList: images.GetAllImages(&cfg.ClusterConfiguration)},
}
return RunChecks(checks, os.Stderr, ignorePreflightErrors)
}
Expand Down

0 comments on commit de80c82

Please sign in to comment.