Skip to content
This repository has been archived by the owner on Mar 28, 2020. It is now read-only.

Add curl image parameter #2092

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add curl image parameter #2092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b258fbb
Add Curl Image parameter
Jun 10, 2019
4c28725
cluster: add option to specify curl image
Jun 10, 2019
806ce68
Merge branch 'master' of https://github.com/atiannicelli/etcd-operator
Jun 10, 2019
dc40303
small fix to pass tests.
Jun 12, 2019
ed03109
add pod policy to call to makeRestoreInitContainers
Jun 14, 2019
1dfe75b
Fixing formatting issues seem during testing.
atiannicelli Jul 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/apis/etcd/v1beta2/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"errors"
"strings"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -162,6 +162,11 @@ type PodPolicy struct {
// '.cluster.local'.
// The default is to not set a cluster domain explicitly.
ClusterDomain string `json:"ClusterDomain"`

// curl init container image. default is tutum/curl.
// pulling tutum/curl:latest requires external access.
// More info: https://github.com/coreos/etcd-operator/issues/1933
CurlImage string `json:"curlImage,omitempty"`
Copy link
Collaborator

@alaypatel07 alaypatel07 Aug 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Should be added to EtcdRestore struct

Copy link
Author

Choose a reason for hiding this comment

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

I am following the example of the "busyboxImage" parameter which specific to etcRestore as well. I see how you could say that it belongs there, but I feel these should stay together. THIS fix is to add the curlImage. If we want another fix to move them both to EtcdRestore I can see that working, but I would rather keep this as is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought the busyboxImage parameter is used for all etcd cluster pods and not just etcd restore

etcd-operator/pkg/util/k8sutil/k8sutil.go

Line 374 in b2a63ce

Image: imageNameBusybox(cs.Pod),

}

// TODO: move this to initializer
Expand Down
18 changes: 14 additions & 4 deletions pkg/util/k8sutil/k8sutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const (
MaxNameLength = 63 - randomSuffixLength - 1

defaultBusyboxImage = "busybox:1.28.0-glibc"
defaultCurlImage = "tutum/curl:latest"

// AnnotationScope annotation name for defining instance scope. Used for specifying cluster wide clusters.
AnnotationScope = "etcd.database.coreos.com/scope"
Expand Down Expand Up @@ -100,11 +101,12 @@ func PVCNameFromMember(memberName string) string {
return memberName
}

func makeRestoreInitContainers(backupURL *url.URL, token, repo, version string, m *etcdutil.Member) []v1.Container {
func makeRestoreInitContainers(backupURL *url.URL, token, repo, version string, m *etcdutil.Member, podPolicy *api.PodPolicy) []v1.Container {
return []v1.Container{
{
Name: "fetch-backup",
Image: "tutum/curl",
Name: "fetch-backup",
//Image default: "tutum/curl:latest",
Image: imageNameCurl(podPolicy),
Command: []string{
"/bin/bash", "-ec",
fmt.Sprintf(`
Expand Down Expand Up @@ -147,6 +149,14 @@ func imageNameBusybox(policy *api.PodPolicy) string {
return defaultBusyboxImage
}

// imageNameCurl returns the default image for curl init container, or the image specified in the PodPolicy
func imageNameCurl(policy *api.PodPolicy) string {
if policy != nil && len(policy.CurlImage) > 0 {
return policy.CurlImage
}
return defaultCurlImage
}

func PodWithNodeSelector(p *v1.Pod, ns map[string]string) *v1.Pod {
p.Spec.NodeSelector = ns
return p
Expand Down Expand Up @@ -261,7 +271,7 @@ func AddEtcdVolumeToPod(pod *v1.Pod, pvc *v1.PersistentVolumeClaim) {

func addRecoveryToPod(pod *v1.Pod, token string, m *etcdutil.Member, cs api.ClusterSpec, backupURL *url.URL) {
pod.Spec.InitContainers = append(pod.Spec.InitContainers,
makeRestoreInitContainers(backupURL, token, cs.Repository, cs.Version, m)...)
makeRestoreInitContainers(backupURL, token, cs.Repository, cs.Version, m, cs.Pod)...)
}

func addOwnerRefToObject(o metav1.Object, r metav1.OwnerReference) {
Expand Down
28 changes: 28 additions & 0 deletions pkg/util/k8sutil/k8sutils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,31 @@ func TestSetBusyboxImageName(t *testing.T) {
t.Errorf("expect image=%s, get=%s", expected, image)
}
}

func TestDefaultCurlImageName(t *testing.T) {
policy := &api.PodPolicy{}
image := imageNameCurl(policy)
expected := defaultCurlImage
if image != expected {
t.Errorf("expect image=%s, get=%s", expected, image)
}
}

func TestDefaultNilCurlImageName(t *testing.T) {
image := imageNameCurl(nil)
expected := defaultCurlImage
if image != expected {
t.Errorf("expect image=%s, get=%s", expected, image)
}
}

func TestSetCurlImageName(t *testing.T) {
policy := &api.PodPolicy{
CurlImage: "myRepo/curl:1.3.2",
}
image := imageNameCurl(policy)
expected := "myRepo/curl:1.3.2"
if image != expected {
t.Errorf("expect image=%s, get=%s", expected, image)
}
}