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

🌱 include deamonsets in list images in clusterctl #4455

Merged
merged 1 commit into from
Apr 13, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 81 additions & 39 deletions cmd/clusterctl/internal/util/objs.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ package util
import (
"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
coreV1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/scheme"
)

const (
deploymentKind = "Deployment"
daemonSetKind = "DaemonSet"
controllerContainerName = "manager"
)

Expand All @@ -37,19 +39,32 @@ func InspectImages(objs []unstructured.Unstructured) ([]string, error) {

for i := range objs {
o := objs[i]
if o.GetKind() == deploymentKind {

var podSpec coreV1.PodSpec

switch o.GetKind() {
case deploymentKind:
d := &appsv1.Deployment{}
if err := scheme.Scheme.Convert(&o, d, nil); err != nil {
return nil, err
}

for _, c := range d.Spec.Template.Spec.Containers {
images = append(images, c.Image)
podSpec = d.Spec.Template.Spec
case daemonSetKind:
d := &appsv1.DaemonSet{}
if err := scheme.Scheme.Convert(&o, d, nil); err != nil {
return nil, err
}
podSpec = d.Spec.Template.Spec
default:
continue
}

for _, c := range d.Spec.Template.Spec.InitContainers {
images = append(images, c.Image)
}
for _, c := range podSpec.Containers {
images = append(images, c.Image)
}

for _, c := range podSpec.InitContainers {
images = append(images, c.Image)
}
}

Expand Down Expand Up @@ -102,45 +117,72 @@ func IsSharedResource(o unstructured.Unstructured) bool {
// NB. The implemented approach is specific for the provider components YAML & for the cert-manager manifest; it is not
// intended to cover all the possible objects used to deploy containers existing in Kubernetes.
func FixImages(objs []unstructured.Unstructured, alterImageFunc func(image string) (string, error)) ([]unstructured.Unstructured, error) {
// look for resources of kind Deployment and alter the image
for i := range objs {
ludusrusso marked this conversation as resolved.
Show resolved Hide resolved
o := &objs[i]
if o.GetKind() != deploymentKind {
continue
if err := fixDeploymentImages(&objs[i], alterImageFunc); err != nil {
return nil, err
}

// Convert Unstructured into a typed object
d := &appsv1.Deployment{}
if err := scheme.Scheme.Convert(o, d, nil); err != nil {
if err := fixDaemonSetImages(&objs[i], alterImageFunc); err != nil {
return nil, err
}
}
return objs, nil
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to return objs here? Seems like we're doing in place updates of the slice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that this is not required but it's part of the function interface!

Copy link
Member

Choose a reason for hiding this comment

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

cc @fabriziopandini We might want to revisit ^

}

// Alter the image
for j := range d.Spec.Template.Spec.Containers {
container := d.Spec.Template.Spec.Containers[j]
image, err := alterImageFunc(container.Image)
if err != nil {
return nil, errors.Wrapf(err, "failed to fix image for container %s in deployment %s", container.Name, d.Name)
}
container.Image = image
d.Spec.Template.Spec.Containers[j] = container
}
func fixDeploymentImages(o *unstructured.Unstructured, alterImageFunc func(image string) (string, error)) error {
if o.GetKind() != deploymentKind {
return nil
}

for j := range d.Spec.Template.Spec.InitContainers {
container := d.Spec.Template.Spec.InitContainers[j]
image, err := alterImageFunc(container.Image)
if err != nil {
return nil, errors.Wrapf(err, "failed to fix image for init container %s in deployment %s", container.Name, d.Name)
}
container.Image = image
d.Spec.Template.Spec.InitContainers[j] = container
}
// Convert Unstructured into a typed object
d := &appsv1.Deployment{}
if err := scheme.Scheme.Convert(o, d, nil); err != nil {
return err
}

// Convert typed object back to Unstructured
if err := scheme.Scheme.Convert(d, o, nil); err != nil {
return nil, err
if err := fixPodSpecImages(&d.Spec.Template.Spec, alterImageFunc); err != nil {
return errors.Wrapf(err, "failed to fix containers in deployment %s", d.Name)
}

// Convert typed object back to Unstructured
return scheme.Scheme.Convert(d, o, nil)
}

func fixDaemonSetImages(o *unstructured.Unstructured, alterImageFunc func(image string) (string, error)) error {
if o.GetKind() != daemonSetKind {
return nil
}

// Convert Unstructured into a typed object
d := &appsv1.DaemonSet{}
if err := scheme.Scheme.Convert(o, d, nil); err != nil {
return err
}

if err := fixPodSpecImages(&d.Spec.Template.Spec, alterImageFunc); err != nil {
return errors.Wrapf(err, "failed to fix containers in deamonSet %s", d.Name)
ludusrusso marked this conversation as resolved.
Show resolved Hide resolved
}
// Convert typed object back to Unstructured
return scheme.Scheme.Convert(d, o, nil)
}

func fixPodSpecImages(podSpec *coreV1.PodSpec, alterImageFunc func(image string) (string, error)) error {
if err := fixContainersImage(podSpec.Containers, alterImageFunc); err != nil {
return errors.Wrapf(err, "failed to fix containers")
}
if err := fixContainersImage(podSpec.InitContainers, alterImageFunc); err != nil {
return errors.Wrapf(err, "failed to fix init containers")
}
return nil
}

func fixContainersImage(containers []coreV1.Container, alterImageFunc func(image string) (string, error)) error {
for j := range containers {
container := &containers[j]
image, err := alterImageFunc(container.Image)
if err != nil {
return errors.Wrapf(err, "failed to fix image for container %s", container.Name)
}
objs[i] = *o
container.Image = image
}
return objs, nil
return nil
}
67 changes: 67 additions & 0 deletions cmd/clusterctl/internal/util/objs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,39 @@ func Test_inspectImages(t *testing.T) {
want: []string{"gcr.io/k8s-staging-cluster-api/cluster-api-controller:master", "gcr.io/k8s-staging-cluster-api/cluster-api-controller:init"},
wantErr: false,
},
{
name: "controller with deamonSet",
args: args{
objs: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": daemonSetKind,
"spec": map[string]interface{}{
"template": map[string]interface{}{
"spec": map[string]interface{}{
"containers": []map[string]interface{}{
{
"name": controllerContainerName,
"image": "gcr.io/k8s-staging-cluster-api/cluster-api-controller:master",
},
},
"initContainers": []map[string]interface{}{
{
"name": controllerContainerName,
"image": "gcr.io/k8s-staging-cluster-api/cluster-api-controller:init",
},
},
},
},
},
},
},
},
},
want: []string{"gcr.io/k8s-staging-cluster-api/cluster-api-controller:master", "gcr.io/k8s-staging-cluster-api/cluster-api-controller:init"},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -188,6 +221,40 @@ func TestFixImages(t *testing.T) {
want: []string{"foo-container-image", "foo-init-container-image"},
wantErr: false,
},
{
name: "fix daemonSet containers images",
args: args{
objs: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": daemonSetKind,
"spec": map[string]interface{}{
"template": map[string]interface{}{
"spec": map[string]interface{}{
"containers": []map[string]interface{}{
{
"image": "container-image",
},
},
"initContainers": []map[string]interface{}{
{
"image": "init-container-image",
},
},
},
},
},
},
},
},
alterImageFunc: func(image string) (string, error) {
return fmt.Sprintf("foo-%s", image), nil
},
},
want: []string{"foo-container-image", "foo-init-container-image"},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down