From c534fbb75ef2ccac8cfee1666768ad9738cad655 Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Tue, 11 Aug 2020 12:21:42 +0200 Subject: [PATCH 01/11] feat(podspec): add possibility to set container image in Deployment --- internal/dao/dp.go | 56 ++++++++++-- internal/dao/types.go | 8 ++ internal/view/dp.go | 8 +- internal/view/dp_test.go | 2 +- internal/view/set_image_extender.go | 136 ++++++++++++++++++++++++++++ 5 files changed, 200 insertions(+), 10 deletions(-) create mode 100644 internal/view/set_image_extender.go diff --git a/internal/dao/dp.go b/internal/dao/dp.go index b4ad9b34..a5fb19c5 100644 --- a/internal/dao/dp.go +++ b/internal/dao/dp.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "github.com/derailed/k9s/internal/client" "github.com/rs/zerolog/log" @@ -18,12 +19,13 @@ import ( ) var ( - _ Accessor = (*Deployment)(nil) - _ Nuker = (*Deployment)(nil) - _ Loggable = (*Deployment)(nil) - _ Restartable = (*Deployment)(nil) - _ Scalable = (*Deployment)(nil) - _ Controller = (*Deployment)(nil) + _ Accessor = (*Deployment)(nil) + _ Nuker = (*Deployment)(nil) + _ Loggable = (*Deployment)(nil) + _ Restartable = (*Deployment)(nil) + _ Scalable = (*Deployment)(nil) + _ Controller = (*Deployment)(nil) + _ ContainsPodSpec = (*Deployment)(nil) ) // Deployment represents a deployment K8s resource. @@ -211,6 +213,48 @@ func (d *Deployment) Scan(ctx context.Context, gvr, fqn string, wait bool) (Refs return refs, nil } +func (d *Deployment) GetPodSpec(path string) (*v1.PodSpec, error) { + dp, err := d.Load(d.Factory, path) + if err != nil { + return nil, err + } + podSpec := dp.Spec.Template.Spec + return &podSpec, nil +} + +func (d *Deployment) SetImages(ctx context.Context, path string, images map[string]string) error { + ns, n := client.Namespaced(path) + auth, err := d.Client().CanI(ns, "apps/v1/deployments", []string{client.PatchVerb}) + if err != nil { + return err + } + if !auth { + return fmt.Errorf("user is not authorized to patch a deployment") + } + var nameStrB strings.Builder + var imageStrB strings.Builder + + for name, image := range images { + nameStrB.WriteString(fmt.Sprintf(`{"name":"%s"},`, name)) + imageStrB.WriteString(fmt.Sprintf(`{"image":"%s","name":"%s"},`, image, name)) + } + namesJson := strings.TrimSuffix(nameStrB.String(), ",") + imagesJson := strings.TrimSuffix(imageStrB.String(), ",") + patchJson := `{"spec":{"template":{"spec":{"$setElementOrder/containers":[` + namesJson + `],"containers":[` + imagesJson + `]}}}}` + dial, err := d.Client().Dial() + if err != nil { + return err + } + _, err = dial.AppsV1().Deployments(ns).Patch( + ctx, + n, + types.StrategicMergePatchType, + []byte(patchJson), + metav1.PatchOptions{}, + ) + return err +} + func hasPVC(spec *v1.PodSpec, name string) bool { for _, v := range spec.Volumes { if v.PersistentVolumeClaim != nil && v.PersistentVolumeClaim.ClaimName == name { diff --git a/internal/dao/types.go b/internal/dao/types.go index ddf671a6..2adfc25e 100644 --- a/internal/dao/types.go +++ b/internal/dao/types.go @@ -146,3 +146,11 @@ type Logger interface { // Logs tails a resource logs. Logs(path string, opts *v1.PodLogOptions) (*restclient.Request, error) } + +type ContainsPodSpec interface { + // Get PodSpec of a resource + GetPodSpec(path string) (*v1.PodSpec, error) + + // Set Images for a resource + SetImages(ctx context.Context, path string, images map[string]string) error +} diff --git a/internal/view/dp.go b/internal/view/dp.go index f5acbcaa..8fc65a93 100644 --- a/internal/view/dp.go +++ b/internal/view/dp.go @@ -21,9 +21,11 @@ func NewDeploy(gvr client.GVR) ResourceViewer { ResourceViewer: NewPortForwardExtender( NewRestartExtender( NewScaleExtender( - NewLogsExtender( - NewBrowser(gvr), - nil, + NewSetImageExtender( + NewLogsExtender( + NewBrowser(gvr), + nil, + ), ), ), ), diff --git a/internal/view/dp_test.go b/internal/view/dp_test.go index d69fa571..c78a9e00 100644 --- a/internal/view/dp_test.go +++ b/internal/view/dp_test.go @@ -13,5 +13,5 @@ func TestDeploy(t *testing.T) { assert.Nil(t, v.Init(makeCtx())) assert.Equal(t, "Deployments", v.Name()) - assert.Equal(t, 12, len(v.Hints())) + assert.Equal(t, 13, len(v.Hints())) } diff --git a/internal/view/set_image_extender.go b/internal/view/set_image_extender.go new file mode 100644 index 00000000..94592a67 --- /dev/null +++ b/internal/view/set_image_extender.go @@ -0,0 +1,136 @@ +package view + +import ( + "context" + "fmt" + "github.com/derailed/k9s/internal/dao" + "github.com/derailed/k9s/internal/ui" + "github.com/derailed/tview" + "github.com/gdamore/tcell" + "github.com/rs/zerolog/log" + corev1 "k8s.io/api/core/v1" +) + +// SetImageExtender adds set image extensions +type SetImageExtender struct { + ResourceViewer +} + +const setImageKey = "setImage" + +func NewSetImageExtender(r ResourceViewer) ResourceViewer { + s := SetImageExtender{ResourceViewer: r} + s.bindKeys(s.Actions()) + + return &s +} + +func (s *SetImageExtender) bindKeys(aa ui.KeyActions) { + aa.Add(ui.KeyActions{ + ui.KeyI: ui.NewKeyAction("SetImage", s.setImageCmd, true), + }) +} + +func (s *SetImageExtender) setImageCmd(evt *tcell.EventKey) *tcell.EventKey { + path := s.GetTable().GetSelectedItem() + if path == "" { + return nil + } + + s.Stop() + defer s.Start() + s.showSetImageDialog(path) + + return nil +} + +func (s *SetImageExtender) showSetImageDialog(path string) { + confirm := tview.NewModalForm("", s.makeSetImageForm(path)) + confirm.SetText(fmt.Sprintf("Set image %s %s", s.GVR(), path)) + confirm.SetDoneFunc(func(int, string) { + s.dismissDialog() + }) + s.App().Content.AddPage(setImageKey, confirm, false, false) + s.App().Content.ShowPage(setImageKey) +} + +func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { + f := s.makeStyledForm() + containers, err := s.getImages(sel) + if err != nil { + s.App().Flash().Err(err) + return nil + } + images := make(map[string]string) + for _, container := range *containers { + f.AddInputField(container.Name, container.Image, 0, nil, func(changed string) { + images[container.Name] = changed + }) + } + + f.AddButton("OK", func() { + defer s.dismissDialog() + + if err != nil { + s.App().Flash().Err(err) + return + } + ctx, cancel := context.WithTimeout(context.Background(), s.App().Conn().Config().CallTimeout()) + defer cancel() + if err := s.setImages(ctx, sel, images); err != nil { + log.Error().Err(err).Msgf("DP %s image update failed", sel) + s.App().Flash().Err(err) + } else { + s.App().Flash().Infof("Resource %s:%s image updated successfully", s.GVR(), sel) + } + }) + + f.AddButton("Cancel", func() { + s.dismissDialog() + }) + + return f +} + +func (s *SetImageExtender) dismissDialog() { + s.App().Content.RemovePage(setImageKey) +} + +func (s *SetImageExtender) makeStyledForm() *tview.Form { + f := tview.NewForm() + f.SetItemPadding(0) + f.SetButtonsAlign(tview.AlignCenter). + SetButtonBackgroundColor(tview.Styles.PrimitiveBackgroundColor). + SetButtonTextColor(tview.Styles.PrimaryTextColor). + SetLabelColor(tcell.ColorAqua). + SetFieldTextColor(tcell.ColorOrange) + return f +} + +func (s *SetImageExtender) getImages(path string) (*[]corev1.Container, error) { + res, err := dao.AccessorFor(s.App().factory, s.GVR()) + if err != nil { + return nil, err + } + podSpecable, ok := res.(dao.ContainsPodSpec) + if !ok { + return nil, fmt.Errorf("expecting a podSpecable resource for %q", s.GVR()) + } + podSpec, err := podSpecable.GetPodSpec(path) + containers := podSpec.Containers + return &containers, nil +} + +func (s *SetImageExtender) setImages(ctx context.Context, path string, images map[string]string) error { + res, err := dao.AccessorFor(s.App().factory, s.GVR()) + if err != nil { + return err + } + + deployment, ok := res.(dao.ContainsPodSpec) + if !ok { + return fmt.Errorf("expecting a scalable resource for %q", s.GVR()) + } + + return deployment.SetImages(ctx, path, images) +} From 8cb9da07b16682a2c5889d409752776d5d2faae2 Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Wed, 12 Aug 2020 21:27:31 +0200 Subject: [PATCH 02/11] feat(dao/dp): use external patch method to create json patch --- internal/dao/dp.go | 15 ++++-------- internal/dao/patch.go | 47 ++++++++++++++++++++++++++++++++++++++ internal/dao/patch_test.go | 39 +++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 internal/dao/patch.go create mode 100644 internal/dao/patch_test.go diff --git a/internal/dao/dp.go b/internal/dao/dp.go index a5fb19c5..13f58bbb 100644 --- a/internal/dao/dp.go +++ b/internal/dao/dp.go @@ -4,8 +4,6 @@ import ( "context" "errors" "fmt" - "strings" - "github.com/derailed/k9s/internal/client" "github.com/rs/zerolog/log" appsv1 "k8s.io/api/apps/v1" @@ -231,16 +229,11 @@ func (d *Deployment) SetImages(ctx context.Context, path string, images map[stri if !auth { return fmt.Errorf("user is not authorized to patch a deployment") } - var nameStrB strings.Builder - var imageStrB strings.Builder - for name, image := range images { - nameStrB.WriteString(fmt.Sprintf(`{"name":"%s"},`, name)) - imageStrB.WriteString(fmt.Sprintf(`{"image":"%s","name":"%s"},`, image, name)) + jsonPatch, err := SetImageJsonPatch(images) + if err != nil { + return err } - namesJson := strings.TrimSuffix(nameStrB.String(), ",") - imagesJson := strings.TrimSuffix(imageStrB.String(), ",") - patchJson := `{"spec":{"template":{"spec":{"$setElementOrder/containers":[` + namesJson + `],"containers":[` + imagesJson + `]}}}}` dial, err := d.Client().Dial() if err != nil { return err @@ -249,7 +242,7 @@ func (d *Deployment) SetImages(ctx context.Context, path string, images map[stri ctx, n, types.StrategicMergePatchType, - []byte(patchJson), + []byte(jsonPatch), metav1.PatchOptions{}, ) return err diff --git a/internal/dao/patch.go b/internal/dao/patch.go new file mode 100644 index 00000000..63e39474 --- /dev/null +++ b/internal/dao/patch.go @@ -0,0 +1,47 @@ +package dao + +import "encoding/json" + +type JsonPatch struct { + Spec Spec `json:"spec"` +} + +type Spec struct { + Template Template `json:"template"` +} + +type Template struct { + Spec ImagesSpec `json:"spec"` +} + +type ImagesSpec struct { + SetElementOrders []Element `json:"$setElementOrder/containers"` + Containers []Element `json:"containers"` +} + +type Element struct { + Image string `json:"image,omitempty"` + Name string `json:"name"` +} + +// Build a json patch string to update PodSpec images +func SetImageJsonPatch(images map[string]string) (string, error) { + elementOrders := make([]Element, 0) + containers := make([]Element, 0) + for key, value := range images { + elementOrders = append(elementOrders, Element{Name: key}) + containers = append(containers, Element{Name: key, Image: value}) + } + jsonPatch := JsonPatch{ + Spec: Spec{ + Template: Template{ + Spec: ImagesSpec{ + SetElementOrders: elementOrders, + Containers: containers, + }, + }, + }, + } + bytes, err := json.Marshal(jsonPatch) + return string(bytes), err +} diff --git a/internal/dao/patch_test.go b/internal/dao/patch_test.go new file mode 100644 index 00000000..f083d2db --- /dev/null +++ b/internal/dao/patch_test.go @@ -0,0 +1,39 @@ +package dao + +import ( + "reflect" + "testing" +) + +func TestSetImageJsonPatch(t *testing.T) { + type args struct { + images map[string]string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "simple", + args: args{ + images: map[string]string{"nginx": "nginx:latest"}, + }, + want: "", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := SetImageJsonPatch(tt.args.images) + if (err != nil) != tt.wantErr { + t.Errorf("SetImageJsonPatch() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("SetImageJsonPatch() got = %v, want %v", got, tt.want) + } + }) + } +} From 41e9d691b773c0d9784f553c72c8eba208ce803e Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Tue, 18 Aug 2020 22:19:07 +0200 Subject: [PATCH 03/11] feat(image extender): set image for init container --- go.mod | 2 +- go.sum | 4 ++ internal/dao/dp.go | 5 +- internal/dao/patch.go | 37 +++++++++----- internal/dao/patch_test.go | 18 ++++--- internal/dao/types.go | 2 +- internal/view/set_image_extender.go | 78 ++++++++++++++++++++++++----- 7 files changed, 109 insertions(+), 37 deletions(-) diff --git a/go.mod b/go.mod index 28043dcc..af18f20c 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/ryanuber/go-glob v1.0.0 // indirect github.com/sahilm/fuzzy v0.1.0 github.com/spf13/cobra v1.0.0 - github.com/stretchr/testify v1.5.1 + github.com/stretchr/testify v1.6.1 golang.org/x/net v0.0.0-20200519113804-d87ec0cfa476 // indirect golang.org/x/sys v0.0.0-20200519105757-fe76b779f299 // indirect golang.org/x/text v0.3.2 diff --git a/go.sum b/go.sum index d30e2137..f61f060b 100644 --- a/go.sum +++ b/go.sum @@ -615,6 +615,8 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/syndtr/gocapability v0.0.0-20170704070218-db04d3cc01c8/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww= github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= @@ -839,6 +841,8 @@ gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= helm.sh/helm/v3 v3.2.0 h1:V12EGAmr2DJ/fWrPo2fPdXWSIXvlXm51vGkQIXMeymE= diff --git a/internal/dao/dp.go b/internal/dao/dp.go index 13f58bbb..4c6431fd 100644 --- a/internal/dao/dp.go +++ b/internal/dao/dp.go @@ -220,7 +220,7 @@ func (d *Deployment) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (d *Deployment) SetImages(ctx context.Context, path string, images map[string]string) error { +func (d *Deployment) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { ns, n := client.Namespaced(path) auth, err := d.Client().CanI(ns, "apps/v1/deployments", []string{client.PatchVerb}) if err != nil { @@ -229,8 +229,7 @@ func (d *Deployment) SetImages(ctx context.Context, path string, images map[stri if !auth { return fmt.Errorf("user is not authorized to patch a deployment") } - - jsonPatch, err := SetImageJsonPatch(images) + jsonPatch, err := SetImageJsonPatch(spec) if err != nil { return err } diff --git a/internal/dao/patch.go b/internal/dao/patch.go index 63e39474..439f79eb 100644 --- a/internal/dao/patch.go +++ b/internal/dao/patch.go @@ -1,6 +1,9 @@ package dao -import "encoding/json" +import ( + "encoding/json" + v1 "k8s.io/api/core/v1" +) type JsonPatch struct { Spec Spec `json:"spec"` @@ -15,8 +18,10 @@ type Template struct { } type ImagesSpec struct { - SetElementOrders []Element `json:"$setElementOrder/containers"` - Containers []Element `json:"containers"` + SetElementOrderContainers []Element `json:"$setElementOrder/containers,omitempty"` + SetElementOrderInitContainers []Element `json:"$setElementOrder/initContainers,omitempty"` + Containers []Element `json:"containers,omitempty"` + InitContainers []Element `json:"initContainers,omitempty"` } type Element struct { @@ -25,19 +30,15 @@ type Element struct { } // Build a json patch string to update PodSpec images -func SetImageJsonPatch(images map[string]string) (string, error) { - elementOrders := make([]Element, 0) - containers := make([]Element, 0) - for key, value := range images { - elementOrders = append(elementOrders, Element{Name: key}) - containers = append(containers, Element{Name: key, Image: value}) - } +func SetImageJsonPatch(spec v1.PodSpec) (string, error) { jsonPatch := JsonPatch{ Spec: Spec{ Template: Template{ Spec: ImagesSpec{ - SetElementOrders: elementOrders, - Containers: containers, + SetElementOrderContainers: extractElements(spec.Containers, false), + Containers: extractElements(spec.Containers, true), + SetElementOrderInitContainers: extractElements(spec.InitContainers, false), + InitContainers: extractElements(spec.InitContainers, true), }, }, }, @@ -45,3 +46,15 @@ func SetImageJsonPatch(images map[string]string) (string, error) { bytes, err := json.Marshal(jsonPatch) return string(bytes), err } + +func extractElements(containers []v1.Container, withImage bool) []Element { + elements := make([]Element, 0) + for _, c := range containers { + if withImage { + elements = append(elements, Element{Name: c.Name, Image: c.Image}) + } else { + elements = append(elements, Element{Name: c.Name}) + } + } + return elements +} diff --git a/internal/dao/patch_test.go b/internal/dao/patch_test.go index f083d2db..15c9183f 100644 --- a/internal/dao/patch_test.go +++ b/internal/dao/patch_test.go @@ -1,13 +1,14 @@ package dao import ( - "reflect" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" "testing" ) func TestSetImageJsonPatch(t *testing.T) { type args struct { - images map[string]string + podSpec v1.PodSpec } tests := []struct { name string @@ -18,22 +19,23 @@ func TestSetImageJsonPatch(t *testing.T) { { name: "simple", args: args{ - images: map[string]string{"nginx": "nginx:latest"}, + podSpec: v1.PodSpec{ + InitContainers: []v1.Container{v1.Container{Image: "busybox:latest", Name: "init"}}, + Containers: []v1.Container{v1.Container{Image: "nginx:latest", Name: "nginx"}}, + }, }, - want: "", + want: `{"spec":{"template":{"spec":{"$setElementOrder/containers":[{"name":"nginx"}],"$setElementOrder/initContainers":[{"name":"init"}],"containers":[{"image":"nginx:latest","name":"nginx"}],"initContainers":[{"image":"busybox:latest","name":"init"}]}}}}`, wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := SetImageJsonPatch(tt.args.images) + got, err := SetImageJsonPatch(tt.args.podSpec) if (err != nil) != tt.wantErr { t.Errorf("SetImageJsonPatch() error = %v, wantErr %v", err, tt.wantErr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("SetImageJsonPatch() got = %v, want %v", got, tt.want) - } + require.JSONEq(t, tt.want, got, "Json strings should be equal") }) } } diff --git a/internal/dao/types.go b/internal/dao/types.go index 2adfc25e..f1c9c5f5 100644 --- a/internal/dao/types.go +++ b/internal/dao/types.go @@ -152,5 +152,5 @@ type ContainsPodSpec interface { GetPodSpec(path string) (*v1.PodSpec, error) // Set Images for a resource - SetImages(ctx context.Context, path string, images map[string]string) error + SetImages(ctx context.Context, path string, spec v1.PodSpec) error } diff --git a/internal/view/set_image_extender.go b/internal/view/set_image_extender.go index 94592a67..8ba5271a 100644 --- a/internal/view/set_image_extender.go +++ b/internal/view/set_image_extender.go @@ -54,17 +54,29 @@ func (s *SetImageExtender) showSetImageDialog(path string) { s.App().Content.ShowPage(setImageKey) } +type ContainerType string + +var runningContainer = ContainerType("Container") +var initContainer = ContainerType("InitContainer") + +type ContainerImage struct { + ContainerType ContainerType + Image string +} + func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { f := s.makeStyledForm() - containers, err := s.getImages(sel) + podSpec, err := s.getPodSpec(sel) + originalImages := getImages(podSpec) + formSubmitResult := make(map[string]ContainerImage, 0) if err != nil { s.App().Flash().Err(err) return nil } - images := make(map[string]string) - for _, container := range *containers { - f.AddInputField(container.Name, container.Image, 0, nil, func(changed string) { - images[container.Name] = changed + for name, containerImage := range originalImages { + f.AddInputField(name, containerImage.Image, 0, nil, func(changed string) { + log.Info().Msgf("changed : %v", changed) + formSubmitResult[name] = ContainerImage{ContainerType: containerImage.ContainerType, Image: changed} }) } @@ -77,14 +89,15 @@ func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { } ctx, cancel := context.WithTimeout(context.Background(), s.App().Conn().Config().CallTimeout()) defer cancel() - if err := s.setImages(ctx, sel, images); err != nil { + podSpecPatch := buildPodSpecPatch(formSubmitResult, originalImages) + if err := s.setImages(ctx, sel, podSpecPatch); err != nil { + log.Error().Err(err).Msgf("DP %s image update failed", sel) s.App().Flash().Err(err) } else { s.App().Flash().Infof("Resource %s:%s image updated successfully", s.GVR(), sel) } }) - f.AddButton("Cancel", func() { s.dismissDialog() }) @@ -92,6 +105,48 @@ func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { return f } +func getImages(podSpec *corev1.PodSpec) map[string]ContainerImage { + results := make(map[string]ContainerImage, 0) + for _, c := range podSpec.Containers { + results[c.Name] = ContainerImage{ + ContainerType: runningContainer, + Image: c.Image, + } + } + for _, c := range podSpec.InitContainers { + results[c.Name] = ContainerImage{ + ContainerType: initContainer, + Image: c.Image, + } + } + return results +} + +func buildPodSpecPatch(formImages map[string]ContainerImage, originalImages map[string]ContainerImage) corev1.PodSpec { + initContainers := make([]corev1.Container, 0) + containers := make([]corev1.Container, 0) + for name, containerImage := range formImages { + if originalImages[name].Image == containerImage.Image { + continue + } + container := corev1.Container{ + Image: containerImage.Image, + Name: name, + } + switch containerImage.ContainerType { + case runningContainer: + containers = append(containers, container) + case initContainer: + initContainers = append(initContainers, container) + } + } + result := corev1.PodSpec{ + Containers: containers, + InitContainers: initContainers, + } + return result +} + func (s *SetImageExtender) dismissDialog() { s.App().Content.RemovePage(setImageKey) } @@ -107,7 +162,7 @@ func (s *SetImageExtender) makeStyledForm() *tview.Form { return f } -func (s *SetImageExtender) getImages(path string) (*[]corev1.Container, error) { +func (s *SetImageExtender) getPodSpec(path string) (*corev1.PodSpec, error) { res, err := dao.AccessorFor(s.App().factory, s.GVR()) if err != nil { return nil, err @@ -117,11 +172,10 @@ func (s *SetImageExtender) getImages(path string) (*[]corev1.Container, error) { return nil, fmt.Errorf("expecting a podSpecable resource for %q", s.GVR()) } podSpec, err := podSpecable.GetPodSpec(path) - containers := podSpec.Containers - return &containers, nil + return podSpec, nil } -func (s *SetImageExtender) setImages(ctx context.Context, path string, images map[string]string) error { +func (s *SetImageExtender) setImages(ctx context.Context, path string, spec corev1.PodSpec) error { res, err := dao.AccessorFor(s.App().factory, s.GVR()) if err != nil { return err @@ -132,5 +186,5 @@ func (s *SetImageExtender) setImages(ctx context.Context, path string, images ma return fmt.Errorf("expecting a scalable resource for %q", s.GVR()) } - return deployment.SetImages(ctx, path, images) + return deployment.SetImages(ctx, path, spec) } From 612148debb5d72a32d3036de0bf5aafb6aa40ba1 Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Thu, 20 Aug 2020 12:41:26 +0200 Subject: [PATCH 04/11] chore: move some code --- internal/view/set_image_extender.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/internal/view/set_image_extender.go b/internal/view/set_image_extender.go index 8ba5271a..fe9b0dc3 100644 --- a/internal/view/set_image_extender.go +++ b/internal/view/set_image_extender.go @@ -16,7 +16,18 @@ type SetImageExtender struct { ResourceViewer } -const setImageKey = "setImage" +type ContainerType string + +type ContainerImage struct { + ContainerType ContainerType + Image string +} + +const ( + setImageKey = "setImage" + runningContainer = ContainerType("Container") + initContainer = ContainerType("InitContainer") +) func NewSetImageExtender(r ResourceViewer) ResourceViewer { s := SetImageExtender{ResourceViewer: r} @@ -54,16 +65,6 @@ func (s *SetImageExtender) showSetImageDialog(path string) { s.App().Content.ShowPage(setImageKey) } -type ContainerType string - -var runningContainer = ContainerType("Container") -var initContainer = ContainerType("InitContainer") - -type ContainerImage struct { - ContainerType ContainerType - Image string -} - func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { f := s.makeStyledForm() podSpec, err := s.getPodSpec(sel) From e75bf0acfa7b3d606fe6a51a9759bbc144de4901 Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Thu, 20 Aug 2020 12:42:29 +0200 Subject: [PATCH 05/11] feat(set_image): implement set_image for daemonset --- internal/dao/ds.go | 47 ++++++++++++++++++++++++++++++++++++++++----- internal/view/ds.go | 4 +++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/internal/dao/ds.go b/internal/dao/ds.go index 78e99fdd..38d94ba6 100644 --- a/internal/dao/ds.go +++ b/internal/dao/ds.go @@ -21,11 +21,12 @@ import ( ) var ( - _ Accessor = (*DaemonSet)(nil) - _ Nuker = (*DaemonSet)(nil) - _ Loggable = (*DaemonSet)(nil) - _ Restartable = (*DaemonSet)(nil) - _ Controller = (*DaemonSet)(nil) + _ Accessor = (*DaemonSet)(nil) + _ Nuker = (*DaemonSet)(nil) + _ Loggable = (*DaemonSet)(nil) + _ Restartable = (*DaemonSet)(nil) + _ Controller = (*DaemonSet)(nil) + _ ContainsPodSpec = (*DaemonSet)(nil) ) // DaemonSet represents a K8s daemonset. @@ -225,6 +226,42 @@ func (d *DaemonSet) Scan(ctx context.Context, gvr, fqn string, wait bool) (Refs, return refs, nil } +func (d *DaemonSet) GetPodSpec(path string) (*v1.PodSpec, error) { + ds, err := d.GetInstance(path) + if err != nil { + return nil, err + } + podSpec := ds.Spec.Template.Spec + return &podSpec, nil +} + +func (d *DaemonSet) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { + ns, n := client.Namespaced(path) + auth, err := d.Client().CanI(ns, "apps/v1/daemonset", []string{client.PatchVerb}) + if err != nil { + return err + } + if !auth { + return fmt.Errorf("user is not authorized to patch a daemonset") + } + jsonPatch, err := SetImageJsonPatch(spec) + if err != nil { + return err + } + dial, err := d.Client().Dial() + if err != nil { + return err + } + _, err = dial.AppsV1().DaemonSets(ns).Patch( + ctx, + n, + types.StrategicMergePatchType, + []byte(jsonPatch), + metav1.PatchOptions{}, + ) + return err +} + // ---------------------------------------------------------------------------- // Helpers... diff --git a/internal/view/ds.go b/internal/view/ds.go index 68334e51..587d8049 100644 --- a/internal/view/ds.go +++ b/internal/view/ds.go @@ -17,7 +17,9 @@ func NewDaemonSet(gvr client.GVR) ResourceViewer { d := DaemonSet{ ResourceViewer: NewPortForwardExtender( NewRestartExtender( - NewLogsExtender(NewBrowser(gvr), nil), + NewSetImageExtender( + NewLogsExtender(NewBrowser(gvr), nil), + ), ), ), } From a892c0a0aac7aa906c814b67953bd867204dc5ad Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Thu, 20 Aug 2020 13:56:58 +0200 Subject: [PATCH 06/11] chore(set_image): fix logs --- internal/view/set_image_extender.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/view/set_image_extender.go b/internal/view/set_image_extender.go index fe9b0dc3..3d015d44 100644 --- a/internal/view/set_image_extender.go +++ b/internal/view/set_image_extender.go @@ -76,7 +76,6 @@ func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { } for name, containerImage := range originalImages { f.AddInputField(name, containerImage.Image, 0, nil, func(changed string) { - log.Info().Msgf("changed : %v", changed) formSubmitResult[name] = ContainerImage{ContainerType: containerImage.ContainerType, Image: changed} }) } @@ -93,7 +92,7 @@ func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { podSpecPatch := buildPodSpecPatch(formSubmitResult, originalImages) if err := s.setImages(ctx, sel, podSpecPatch); err != nil { - log.Error().Err(err).Msgf("DP %s image update failed", sel) + log.Error().Err(err).Msgf("PodSpec %s image update failed", sel) s.App().Flash().Err(err) } else { s.App().Flash().Infof("Resource %s:%s image updated successfully", s.GVR(), sel) From b00c77f477f63682b29d99b894112ee6857dee18 Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Thu, 20 Aug 2020 13:58:06 +0200 Subject: [PATCH 07/11] feat(set_image): add set image feature for pod --- internal/dao/dp.go | 2 +- internal/dao/ds.go | 2 +- internal/dao/patch.go | 34 ++++++++++++++++++--------- internal/dao/patch_test.go | 40 +++++++++++++++++++++++++++++--- internal/dao/pod.go | 47 ++++++++++++++++++++++++++++++++++---- internal/view/pod.go | 4 +++- 6 files changed, 108 insertions(+), 21 deletions(-) diff --git a/internal/dao/dp.go b/internal/dao/dp.go index 4c6431fd..a732b6bf 100644 --- a/internal/dao/dp.go +++ b/internal/dao/dp.go @@ -229,7 +229,7 @@ func (d *Deployment) SetImages(ctx context.Context, path string, spec v1.PodSpec if !auth { return fmt.Errorf("user is not authorized to patch a deployment") } - jsonPatch, err := SetImageJsonPatch(spec) + jsonPatch, err := GetTemplateJsonPatch(spec) if err != nil { return err } diff --git a/internal/dao/ds.go b/internal/dao/ds.go index 38d94ba6..c5aae145 100644 --- a/internal/dao/ds.go +++ b/internal/dao/ds.go @@ -244,7 +244,7 @@ func (d *DaemonSet) SetImages(ctx context.Context, path string, spec v1.PodSpec) if !auth { return fmt.Errorf("user is not authorized to patch a daemonset") } - jsonPatch, err := SetImageJsonPatch(spec) + jsonPatch, err := GetTemplateJsonPatch(spec) if err != nil { return err } diff --git a/internal/dao/patch.go b/internal/dao/patch.go index 439f79eb..dc9bbf2a 100644 --- a/internal/dao/patch.go +++ b/internal/dao/patch.go @@ -10,10 +10,10 @@ type JsonPatch struct { } type Spec struct { - Template Template `json:"template"` + Template PodSpec `json:"template"` } -type Template struct { +type PodSpec struct { Spec ImagesSpec `json:"spec"` } @@ -30,23 +30,35 @@ type Element struct { } // Build a json patch string to update PodSpec images -func SetImageJsonPatch(spec v1.PodSpec) (string, error) { +func GetTemplateJsonPatch(spec v1.PodSpec) (string, error) { jsonPatch := JsonPatch{ Spec: Spec{ - Template: Template{ - Spec: ImagesSpec{ - SetElementOrderContainers: extractElements(spec.Containers, false), - Containers: extractElements(spec.Containers, true), - SetElementOrderInitContainers: extractElements(spec.InitContainers, false), - InitContainers: extractElements(spec.InitContainers, true), - }, - }, + Template: getPatchPodSpec(spec), }, } bytes, err := json.Marshal(jsonPatch) return string(bytes), err } +func GetJsonPatch(spec v1.PodSpec) (string, error) { + podSpec := getPatchPodSpec(spec) + bytes, err := json.Marshal(podSpec) + return string(bytes), err +} + +func getPatchPodSpec(spec v1.PodSpec) PodSpec { + podSpec := PodSpec{ + Spec: ImagesSpec{ + SetElementOrderContainers: extractElements(spec.Containers, false), + Containers: extractElements(spec.Containers, true), + SetElementOrderInitContainers: extractElements(spec.InitContainers, false), + InitContainers: extractElements(spec.InitContainers, true), + }, + } + + return podSpec +} + func extractElements(containers []v1.Container, withImage bool) []Element { elements := make([]Element, 0) for _, c := range containers { diff --git a/internal/dao/patch_test.go b/internal/dao/patch_test.go index 15c9183f..83428d9b 100644 --- a/internal/dao/patch_test.go +++ b/internal/dao/patch_test.go @@ -6,7 +6,7 @@ import ( "testing" ) -func TestSetImageJsonPatch(t *testing.T) { +func TestGetTemplateJsonPatch(t *testing.T) { type args struct { podSpec v1.PodSpec } @@ -30,9 +30,43 @@ func TestSetImageJsonPatch(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := SetImageJsonPatch(tt.args.podSpec) + got, err := GetTemplateJsonPatch(tt.args.podSpec) if (err != nil) != tt.wantErr { - t.Errorf("SetImageJsonPatch() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GetTemplateJsonPatch() error = %v, wantErr %v", err, tt.wantErr) + return + } + require.JSONEq(t, tt.want, got, "Json strings should be equal") + }) + } +} + +func TestGetJsonPatch(t *testing.T) { + type args struct { + podSpec v1.PodSpec + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "simple", + args: args{ + podSpec: v1.PodSpec{ + InitContainers: []v1.Container{v1.Container{Image: "busybox:latest", Name: "init"}}, + Containers: []v1.Container{v1.Container{Image: "nginx:latest", Name: "nginx"}}, + }, + }, + want: `{"spec":{"$setElementOrder/containers":[{"name":"nginx"}],"$setElementOrder/initContainers":[{"name":"init"}],"containers":[{"image":"nginx:latest","name":"nginx"}],"initContainers":[{"image":"busybox:latest","name":"init"}]}}`, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetJsonPatch(tt.args.podSpec) + if (err != nil) != tt.wantErr { + t.Errorf("GetTemplateJsonPatch() error = %v, wantErr %v", err, tt.wantErr) return } require.JSONEq(t, tt.want, got, "Json strings should be equal") diff --git a/internal/dao/pod.go b/internal/dao/pod.go index f607341f..234804e2 100644 --- a/internal/dao/pod.go +++ b/internal/dao/pod.go @@ -18,15 +18,17 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" restclient "k8s.io/client-go/rest" mv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1" ) var ( - _ Accessor = (*Pod)(nil) - _ Nuker = (*Pod)(nil) - _ Loggable = (*Pod)(nil) - _ Controller = (*Pod)(nil) + _ Accessor = (*Pod)(nil) + _ Nuker = (*Pod)(nil) + _ Loggable = (*Pod)(nil) + _ Controller = (*Pod)(nil) + _ ContainsPodSpec = (*Pod)(nil) ) const ( @@ -455,3 +457,40 @@ func in(ll []string, s string) bool { } return false } + +func (p *Pod) GetPodSpec(path string) (*v1.PodSpec, error) { + pod, err := p.GetInstance(path) + if err != nil { + return nil, err + } + podSpec := pod.Spec + return &podSpec, nil +} + +func (p *Pod) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { + ns, n := client.Namespaced(path) + auth, err := p.Client().CanI(ns, "v1/pod", []string{client.PatchVerb}) + if err != nil { + return err + } + if !auth { + return fmt.Errorf("user is not authorized to patch a deployment") + } + jsonPatch, err := GetJsonPatch(spec) + if err != nil { + return err + } + dial, err := p.Client().Dial() + if err != nil { + return err + } + log.Info().Msgf("jsonPatch : %v", jsonPatch) + _, err = dial.CoreV1().Pods(ns).Patch( + ctx, + n, + types.StrategicMergePatchType, + []byte(jsonPatch), + metav1.PatchOptions{}, + ) + return err +} diff --git a/internal/view/pod.go b/internal/view/pod.go index c8b12780..1b458cd5 100644 --- a/internal/view/pod.go +++ b/internal/view/pod.go @@ -29,7 +29,9 @@ type Pod struct { func NewPod(gvr client.GVR) ResourceViewer { p := Pod{} p.ResourceViewer = NewPortForwardExtender( - NewLogsExtender(NewBrowser(gvr), p.selectedContainer), + NewSetImageExtender( + NewLogsExtender(NewBrowser(gvr), p.selectedContainer), + ), ) p.SetBindKeysFn(p.bindKeys) p.GetTable().SetEnterFn(p.showContainers) From 3905ef7d022d53dbdbec16a03b4181300324b702 Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Thu, 20 Aug 2020 21:57:41 +0200 Subject: [PATCH 08/11] feat(set_image): add set image feature for statefulset --- internal/dao/sts.go | 50 ++++++++++++++++++++++++++++++++++++++------ internal/view/sts.go | 4 +++- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/internal/dao/sts.go b/internal/dao/sts.go index b0ea2b0a..b22e1733 100644 --- a/internal/dao/sts.go +++ b/internal/dao/sts.go @@ -9,6 +9,7 @@ import ( "github.com/derailed/k9s/internal/client" "github.com/rs/zerolog/log" appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -18,12 +19,13 @@ import ( ) var ( - _ Accessor = (*StatefulSet)(nil) - _ Nuker = (*StatefulSet)(nil) - _ Loggable = (*StatefulSet)(nil) - _ Restartable = (*StatefulSet)(nil) - _ Scalable = (*StatefulSet)(nil) - _ Controller = (*StatefulSet)(nil) + _ Accessor = (*StatefulSet)(nil) + _ Nuker = (*StatefulSet)(nil) + _ Loggable = (*StatefulSet)(nil) + _ Restartable = (*StatefulSet)(nil) + _ Scalable = (*StatefulSet)(nil) + _ Controller = (*StatefulSet)(nil) + _ ContainsPodSpec = (*StatefulSet)(nil) ) // StatefulSet represents a K8s sts. @@ -219,3 +221,39 @@ func (s *StatefulSet) Scan(ctx context.Context, gvr, fqn string, wait bool) (Ref return refs, nil } + +func (s *StatefulSet) GetPodSpec(path string) (*v1.PodSpec, error) { + sts, err := s.getStatefulSet(path) + if err != nil { + return nil, err + } + podSpec := sts.Spec.Template.Spec + return &podSpec, nil +} + +func (s *StatefulSet) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { + ns, n := client.Namespaced(path) + auth, err := s.Client().CanI(ns, "apps/v1/statefulset", []string{client.PatchVerb}) + if err != nil { + return err + } + if !auth { + return fmt.Errorf("user is not authorized to patch a statefulset") + } + jsonPatch, err := GetTemplateJsonPatch(spec) + if err != nil { + return err + } + dial, err := s.Client().Dial() + if err != nil { + return err + } + _, err = dial.AppsV1().StatefulSets(ns).Patch( + ctx, + n, + types.StrategicMergePatchType, + []byte(jsonPatch), + metav1.PatchOptions{}, + ) + return err +} diff --git a/internal/view/sts.go b/internal/view/sts.go index 5a66a78b..6932319d 100644 --- a/internal/view/sts.go +++ b/internal/view/sts.go @@ -21,7 +21,9 @@ func NewStatefulSet(gvr client.GVR) ResourceViewer { ResourceViewer: NewPortForwardExtender( NewRestartExtender( NewScaleExtender( - NewLogsExtender(NewBrowser(gvr), nil), + NewSetImageExtender( + NewLogsExtender(NewBrowser(gvr), nil), + ), ), ), ), From 7665af1c6fcd9c2691a5a0299270ee846821143f Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Thu, 20 Aug 2020 22:07:37 +0200 Subject: [PATCH 09/11] chore: tiny changes --- internal/dao/pod.go | 1 - internal/view/set_image_extender.go | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/dao/pod.go b/internal/dao/pod.go index 234804e2..c338065c 100644 --- a/internal/dao/pod.go +++ b/internal/dao/pod.go @@ -484,7 +484,6 @@ func (p *Pod) SetImages(ctx context.Context, path string, spec v1.PodSpec) error if err != nil { return err } - log.Info().Msgf("jsonPatch : %v", jsonPatch) _, err = dial.CoreV1().Pods(ns).Patch( ctx, n, diff --git a/internal/view/set_image_extender.go b/internal/view/set_image_extender.go index 3d015d44..d1ce216c 100644 --- a/internal/view/set_image_extender.go +++ b/internal/view/set_image_extender.go @@ -167,11 +167,11 @@ func (s *SetImageExtender) getPodSpec(path string) (*corev1.PodSpec, error) { if err != nil { return nil, err } - podSpecable, ok := res.(dao.ContainsPodSpec) + resourceWPodSpec, ok := res.(dao.ContainsPodSpec) if !ok { - return nil, fmt.Errorf("expecting a podSpecable resource for %q", s.GVR()) + return nil, fmt.Errorf("expecting a resourceWPodSpec resource for %q", s.GVR()) } - podSpec, err := podSpecable.GetPodSpec(path) + podSpec, err := resourceWPodSpec.GetPodSpec(path) return podSpec, nil } @@ -181,10 +181,10 @@ func (s *SetImageExtender) setImages(ctx context.Context, path string, spec core return err } - deployment, ok := res.(dao.ContainsPodSpec) + resourceWPodSpec, ok := res.(dao.ContainsPodSpec) if !ok { return fmt.Errorf("expecting a scalable resource for %q", s.GVR()) } - return deployment.SetImages(ctx, path, spec) + return resourceWPodSpec.SetImages(ctx, path, spec) } From 6f9f9ccea52550d83f9b7105d4a8599b8f9873fc Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Mon, 28 Sep 2020 13:55:50 +0200 Subject: [PATCH 10/11] feat: add error if try to set image on managed pod remove usage of v1.PodSpec for patch resources --- internal/dao/dp.go | 4 +- internal/dao/ds.go | 4 +- internal/dao/patch.go | 36 +++++++-------- internal/dao/patch_test.go | 43 ++++++++--------- internal/dao/pod.go | 28 ++++++++++- internal/dao/sts.go | 4 +- internal/dao/types.go | 2 +- internal/view/ds_test.go | 2 +- internal/view/help_test.go | 2 +- internal/view/pod_test.go | 2 +- internal/view/set_image_extender.go | 72 ++++++++++------------------- internal/view/sts_test.go | 2 +- 12 files changed, 96 insertions(+), 105 deletions(-) diff --git a/internal/dao/dp.go b/internal/dao/dp.go index a732b6bf..89a8f506 100644 --- a/internal/dao/dp.go +++ b/internal/dao/dp.go @@ -220,7 +220,7 @@ func (d *Deployment) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (d *Deployment) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { +func (d *Deployment) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { ns, n := client.Namespaced(path) auth, err := d.Client().CanI(ns, "apps/v1/deployments", []string{client.PatchVerb}) if err != nil { @@ -229,7 +229,7 @@ func (d *Deployment) SetImages(ctx context.Context, path string, spec v1.PodSpec if !auth { return fmt.Errorf("user is not authorized to patch a deployment") } - jsonPatch, err := GetTemplateJsonPatch(spec) + jsonPatch, err := GetTemplateJsonPatch(containersPatch, initContainersPatch) if err != nil { return err } diff --git a/internal/dao/ds.go b/internal/dao/ds.go index c5aae145..e01a9de7 100644 --- a/internal/dao/ds.go +++ b/internal/dao/ds.go @@ -235,7 +235,7 @@ func (d *DaemonSet) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (d *DaemonSet) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { +func (d *DaemonSet) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { ns, n := client.Namespaced(path) auth, err := d.Client().CanI(ns, "apps/v1/daemonset", []string{client.PatchVerb}) if err != nil { @@ -244,7 +244,7 @@ func (d *DaemonSet) SetImages(ctx context.Context, path string, spec v1.PodSpec) if !auth { return fmt.Errorf("user is not authorized to patch a daemonset") } - jsonPatch, err := GetTemplateJsonPatch(spec) + jsonPatch, err := GetTemplateJsonPatch(containersPatch, initContainersPatch) if err != nil { return err } diff --git a/internal/dao/patch.go b/internal/dao/patch.go index dc9bbf2a..f158777d 100644 --- a/internal/dao/patch.go +++ b/internal/dao/patch.go @@ -2,7 +2,6 @@ package dao import ( "encoding/json" - v1 "k8s.io/api/core/v1" ) type JsonPatch struct { @@ -30,43 +29,40 @@ type Element struct { } // Build a json patch string to update PodSpec images -func GetTemplateJsonPatch(spec v1.PodSpec) (string, error) { +func GetTemplateJsonPatch(containersPatch map[string]string, initContainersPatch map[string]string) (string, error) { jsonPatch := JsonPatch{ Spec: Spec{ - Template: getPatchPodSpec(spec), + Template: getPatchPodSpec(containersPatch, initContainersPatch), }, } bytes, err := json.Marshal(jsonPatch) return string(bytes), err } -func GetJsonPatch(spec v1.PodSpec) (string, error) { - podSpec := getPatchPodSpec(spec) +func GetJsonPatch(containersPatch map[string]string, initContainersPatch map[string]string) (string, error) { + podSpec := getPatchPodSpec(containersPatch, initContainersPatch) bytes, err := json.Marshal(podSpec) return string(bytes), err } -func getPatchPodSpec(spec v1.PodSpec) PodSpec { +func getPatchPodSpec(containersPatch map[string]string, initContainersPatch map[string]string) PodSpec { + elementsOrders, elements := extractElements(containersPatch) + initElementsOrders, initElements := extractElements(initContainersPatch) podSpec := PodSpec{ Spec: ImagesSpec{ - SetElementOrderContainers: extractElements(spec.Containers, false), - Containers: extractElements(spec.Containers, true), - SetElementOrderInitContainers: extractElements(spec.InitContainers, false), - InitContainers: extractElements(spec.InitContainers, true), + SetElementOrderContainers: elementsOrders, + Containers: elements, + SetElementOrderInitContainers: initElementsOrders, + InitContainers: initElements, }, } - return podSpec } -func extractElements(containers []v1.Container, withImage bool) []Element { - elements := make([]Element, 0) - for _, c := range containers { - if withImage { - elements = append(elements, Element{Name: c.Name, Image: c.Image}) - } else { - elements = append(elements, Element{Name: c.Name}) - } +func extractElements(containers map[string]string) (elementsOrders []Element, elements []Element) { + for name, image := range containers { + elementsOrders = append(elementsOrders, Element{Name: name}) + elements = append(elements, Element{Name: name, Image: image}) } - return elements + return elementsOrders, elements } diff --git a/internal/dao/patch_test.go b/internal/dao/patch_test.go index 83428d9b..9086514a 100644 --- a/internal/dao/patch_test.go +++ b/internal/dao/patch_test.go @@ -2,35 +2,31 @@ package dao import ( "github.com/stretchr/testify/require" - v1 "k8s.io/api/core/v1" "testing" ) func TestGetTemplateJsonPatch(t *testing.T) { type args struct { - podSpec v1.PodSpec + containers map[string]string + initContainers map[string]string } - tests := []struct { - name string + tests := map[string]struct { args args want string wantErr bool }{ - { - name: "simple", + "simple": { args: args{ - podSpec: v1.PodSpec{ - InitContainers: []v1.Container{v1.Container{Image: "busybox:latest", Name: "init"}}, - Containers: []v1.Container{v1.Container{Image: "nginx:latest", Name: "nginx"}}, - }, + initContainers: map[string]string{"init": "busybox:latest"}, + containers: map[string]string{"nginx": "nginx:latest"}, }, want: `{"spec":{"template":{"spec":{"$setElementOrder/containers":[{"name":"nginx"}],"$setElementOrder/initContainers":[{"name":"init"}],"containers":[{"image":"nginx:latest","name":"nginx"}],"initContainers":[{"image":"busybox:latest","name":"init"}]}}}}`, wantErr: false, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := GetTemplateJsonPatch(tt.args.podSpec) + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := GetTemplateJsonPatch(tt.args.containers, tt.args.initContainers) if (err != nil) != tt.wantErr { t.Errorf("GetTemplateJsonPatch() error = %v, wantErr %v", err, tt.wantErr) return @@ -42,29 +38,26 @@ func TestGetTemplateJsonPatch(t *testing.T) { func TestGetJsonPatch(t *testing.T) { type args struct { - podSpec v1.PodSpec + containers map[string]string + initContainers map[string]string } - tests := []struct { - name string + tests := map[string]struct { args args want string wantErr bool }{ - { - name: "simple", + "simple": { args: args{ - podSpec: v1.PodSpec{ - InitContainers: []v1.Container{v1.Container{Image: "busybox:latest", Name: "init"}}, - Containers: []v1.Container{v1.Container{Image: "nginx:latest", Name: "nginx"}}, - }, + initContainers: map[string]string{"init": "busybox:latest"}, + containers: map[string]string{"nginx": "nginx:latest"}, }, want: `{"spec":{"$setElementOrder/containers":[{"name":"nginx"}],"$setElementOrder/initContainers":[{"name":"init"}],"containers":[{"image":"nginx:latest","name":"nginx"}],"initContainers":[{"image":"busybox:latest","name":"init"}]}}`, wantErr: false, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := GetJsonPatch(tt.args.podSpec) + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := GetJsonPatch(tt.args.containers, tt.args.initContainers) if (err != nil) != tt.wantErr { t.Errorf("GetTemplateJsonPatch() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/internal/dao/pod.go b/internal/dao/pod.go index c338065c..cce528a5 100644 --- a/internal/dao/pod.go +++ b/internal/dao/pod.go @@ -467,7 +467,7 @@ func (p *Pod) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (p *Pod) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { +func (p *Pod) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { ns, n := client.Namespaced(path) auth, err := p.Client().CanI(ns, "v1/pod", []string{client.PatchVerb}) if err != nil { @@ -476,7 +476,13 @@ func (p *Pod) SetImages(ctx context.Context, path string, spec v1.PodSpec) error if !auth { return fmt.Errorf("user is not authorized to patch a deployment") } - jsonPatch, err := GetJsonPatch(spec) + if manager, isManaged, err := p.isControlled(path); isManaged { + if err != nil { + return err + } + return fmt.Errorf("Unable to set image. This pod is managed by %s. Please set the image on the controller", *manager) + } + jsonPatch, err := GetJsonPatch(containersPatch, initContainersPatch) if err != nil { return err } @@ -493,3 +499,21 @@ func (p *Pod) SetImages(ctx context.Context, path string, spec v1.PodSpec) error ) return err } + +func (p *Pod) isControlled(path string) (*string, bool, error) { + pod, err := p.GetInstance(path) + if err != nil { + return nil, false, err + } + references := pod.GetObjectMeta().GetOwnerReferences() + if len(references) != 0 && *references[0].Controller == true { + return getController(references[0]), true, nil + } + return nil, false, nil +} + +func getController(reference metav1.OwnerReference) *string { + var result string + result = fmt.Sprintf("%s/%s", reference.Kind, reference.Name) + return &result +} diff --git a/internal/dao/sts.go b/internal/dao/sts.go index b22e1733..80f36bf1 100644 --- a/internal/dao/sts.go +++ b/internal/dao/sts.go @@ -231,7 +231,7 @@ func (s *StatefulSet) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (s *StatefulSet) SetImages(ctx context.Context, path string, spec v1.PodSpec) error { +func (s *StatefulSet) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { ns, n := client.Namespaced(path) auth, err := s.Client().CanI(ns, "apps/v1/statefulset", []string{client.PatchVerb}) if err != nil { @@ -240,7 +240,7 @@ func (s *StatefulSet) SetImages(ctx context.Context, path string, spec v1.PodSpe if !auth { return fmt.Errorf("user is not authorized to patch a statefulset") } - jsonPatch, err := GetTemplateJsonPatch(spec) + jsonPatch, err := GetTemplateJsonPatch(containersPatch, initContainersPatch) if err != nil { return err } diff --git a/internal/dao/types.go b/internal/dao/types.go index f1c9c5f5..d395f610 100644 --- a/internal/dao/types.go +++ b/internal/dao/types.go @@ -152,5 +152,5 @@ type ContainsPodSpec interface { GetPodSpec(path string) (*v1.PodSpec, error) // Set Images for a resource - SetImages(ctx context.Context, path string, spec v1.PodSpec) error + SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error } diff --git a/internal/view/ds_test.go b/internal/view/ds_test.go index 43d45f8b..0726a3c5 100644 --- a/internal/view/ds_test.go +++ b/internal/view/ds_test.go @@ -13,5 +13,5 @@ func TestDaemonSet(t *testing.T) { assert.Nil(t, v.Init(makeCtx())) assert.Equal(t, "DaemonSets", v.Name()) - assert.Equal(t, 13, len(v.Hints())) + assert.Equal(t, 14, len(v.Hints())) } diff --git a/internal/view/help_test.go b/internal/view/help_test.go index dcb07bf1..dda25ddb 100644 --- a/internal/view/help_test.go +++ b/internal/view/help_test.go @@ -21,7 +21,7 @@ func TestHelp(t *testing.T) { v := view.NewHelp() assert.Nil(t, v.Init(ctx)) - assert.Equal(t, 24, v.GetRowCount()) + assert.Equal(t, 25, v.GetRowCount()) assert.Equal(t, 8, v.GetColumnCount()) assert.Equal(t, "", strings.TrimSpace(v.GetCell(1, 0).Text)) assert.Equal(t, "Attach", strings.TrimSpace(v.GetCell(1, 1).Text)) diff --git a/internal/view/pod_test.go b/internal/view/pod_test.go index f33f312b..bf1dd707 100644 --- a/internal/view/pod_test.go +++ b/internal/view/pod_test.go @@ -16,7 +16,7 @@ func TestPodNew(t *testing.T) { assert.Nil(t, po.Init(makeCtx())) assert.Equal(t, "Pods", po.Name()) - assert.Equal(t, 23, len(po.Hints())) + assert.Equal(t, 24, len(po.Hints())) } // Helpers... diff --git a/internal/view/set_image_extender.go b/internal/view/set_image_extender.go index d1ce216c..5970c32e 100644 --- a/internal/view/set_image_extender.go +++ b/internal/view/set_image_extender.go @@ -68,17 +68,15 @@ func (s *SetImageExtender) showSetImageDialog(path string) { func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { f := s.makeStyledForm() podSpec, err := s.getPodSpec(sel) - originalImages := getImages(podSpec) - formSubmitResult := make(map[string]ContainerImage, 0) + containers, initContainers := getImages(podSpec) + containersResult := make(map[string]string) + initContainersResult := make(map[string]string) if err != nil { s.App().Flash().Err(err) return nil } - for name, containerImage := range originalImages { - f.AddInputField(name, containerImage.Image, 0, nil, func(changed string) { - formSubmitResult[name] = ContainerImage{ContainerType: containerImage.ContainerType, Image: changed} - }) - } + addInputField(f, &containers, &containersResult) + addInputField(f, &initContainers, &initContainersResult) f.AddButton("OK", func() { defer s.dismissDialog() @@ -89,9 +87,7 @@ func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { } ctx, cancel := context.WithTimeout(context.Background(), s.App().Conn().Config().CallTimeout()) defer cancel() - podSpecPatch := buildPodSpecPatch(formSubmitResult, originalImages) - if err := s.setImages(ctx, sel, podSpecPatch); err != nil { - + if err := s.setImages(ctx, sel, containersResult, initContainersResult); err != nil { log.Error().Err(err).Msgf("PodSpec %s image update failed", sel) s.App().Flash().Err(err) } else { @@ -105,46 +101,28 @@ func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { return f } -func getImages(podSpec *corev1.PodSpec) map[string]ContainerImage { - results := make(map[string]ContainerImage, 0) - for _, c := range podSpec.Containers { - results[c.Name] = ContainerImage{ - ContainerType: runningContainer, - Image: c.Image, - } +func addInputField(f *tview.Form, containers *map[string]string, containersResult *map[string]string) { + for name, image := range *containers { + f.AddInputField(name, image, 0, nil, func(changed string) { + if changed == image { + delete(*containersResult, name) + } else { + (*containersResult)[name] = changed + } + }) } - for _, c := range podSpec.InitContainers { - results[c.Name] = ContainerImage{ - ContainerType: initContainer, - Image: c.Image, - } - } - return results } -func buildPodSpecPatch(formImages map[string]ContainerImage, originalImages map[string]ContainerImage) corev1.PodSpec { - initContainers := make([]corev1.Container, 0) - containers := make([]corev1.Container, 0) - for name, containerImage := range formImages { - if originalImages[name].Image == containerImage.Image { - continue - } - container := corev1.Container{ - Image: containerImage.Image, - Name: name, - } - switch containerImage.ContainerType { - case runningContainer: - containers = append(containers, container) - case initContainer: - initContainers = append(initContainers, container) - } +func getImages(podSpec *corev1.PodSpec) (map[string]string, map[string]string) { + containers := make(map[string]string) + initContainers := make(map[string]string) + for _, c := range podSpec.Containers { + containers[c.Name] = c.Image } - result := corev1.PodSpec{ - Containers: containers, - InitContainers: initContainers, + for _, c := range podSpec.InitContainers { + initContainers[c.Name] = c.Image } - return result + return containers, initContainers } func (s *SetImageExtender) dismissDialog() { @@ -175,7 +153,7 @@ func (s *SetImageExtender) getPodSpec(path string) (*corev1.PodSpec, error) { return podSpec, nil } -func (s *SetImageExtender) setImages(ctx context.Context, path string, spec corev1.PodSpec) error { +func (s *SetImageExtender) setImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { res, err := dao.AccessorFor(s.App().factory, s.GVR()) if err != nil { return err @@ -186,5 +164,5 @@ func (s *SetImageExtender) setImages(ctx context.Context, path string, spec core return fmt.Errorf("expecting a scalable resource for %q", s.GVR()) } - return resourceWPodSpec.SetImages(ctx, path, spec) + return resourceWPodSpec.SetImages(ctx, path, containersPatch, initContainersPatch) } diff --git a/internal/view/sts_test.go b/internal/view/sts_test.go index 30a5de22..6b737490 100644 --- a/internal/view/sts_test.go +++ b/internal/view/sts_test.go @@ -13,5 +13,5 @@ func TestStatefulSetNew(t *testing.T) { assert.Nil(t, s.Init(makeCtx())) assert.Equal(t, "StatefulSets", s.Name()) - assert.Equal(t, 11, len(s.Hints())) + assert.Equal(t, 12, len(s.Hints())) } From 27b1bba671d427037ffc8a674c54c1bbe37e4b06 Mon Sep 17 00:00:00 2001 From: Antoine Meausoone Date: Thu, 15 Oct 2020 22:50:11 +0200 Subject: [PATCH 11/11] fix: replace unordered map by ImageSpec array fix comments issues Co-authored-by: Sylvain Dumas --- internal/dao/dp.go | 6 +- internal/dao/ds.go | 6 +- internal/dao/patch.go | 46 ++++++++------ internal/dao/patch_test.go | 50 ++++++++++----- internal/dao/pod.go | 22 +++---- internal/dao/sts.go | 6 +- internal/dao/types.go | 2 +- internal/view/set_image_extender.go | 95 +++++++++++++++-------------- 8 files changed, 131 insertions(+), 102 deletions(-) diff --git a/internal/dao/dp.go b/internal/dao/dp.go index 89a8f506..78a430fe 100644 --- a/internal/dao/dp.go +++ b/internal/dao/dp.go @@ -220,7 +220,7 @@ func (d *Deployment) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (d *Deployment) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { +func (d *Deployment) SetImages(ctx context.Context, path string, imageSpecs ImageSpecs) error { ns, n := client.Namespaced(path) auth, err := d.Client().CanI(ns, "apps/v1/deployments", []string{client.PatchVerb}) if err != nil { @@ -229,7 +229,7 @@ func (d *Deployment) SetImages(ctx context.Context, path string, containersPatch if !auth { return fmt.Errorf("user is not authorized to patch a deployment") } - jsonPatch, err := GetTemplateJsonPatch(containersPatch, initContainersPatch) + jsonPatch, err := GetTemplateJsonPatch(imageSpecs) if err != nil { return err } @@ -241,7 +241,7 @@ func (d *Deployment) SetImages(ctx context.Context, path string, containersPatch ctx, n, types.StrategicMergePatchType, - []byte(jsonPatch), + jsonPatch, metav1.PatchOptions{}, ) return err diff --git a/internal/dao/ds.go b/internal/dao/ds.go index e01a9de7..5c0232ae 100644 --- a/internal/dao/ds.go +++ b/internal/dao/ds.go @@ -235,7 +235,7 @@ func (d *DaemonSet) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (d *DaemonSet) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { +func (d *DaemonSet) SetImages(ctx context.Context, path string, imageSpecs ImageSpecs) error { ns, n := client.Namespaced(path) auth, err := d.Client().CanI(ns, "apps/v1/daemonset", []string{client.PatchVerb}) if err != nil { @@ -244,7 +244,7 @@ func (d *DaemonSet) SetImages(ctx context.Context, path string, containersPatch if !auth { return fmt.Errorf("user is not authorized to patch a daemonset") } - jsonPatch, err := GetTemplateJsonPatch(containersPatch, initContainersPatch) + jsonPatch, err := GetTemplateJsonPatch(imageSpecs) if err != nil { return err } @@ -256,7 +256,7 @@ func (d *DaemonSet) SetImages(ctx context.Context, path string, containersPatch ctx, n, types.StrategicMergePatchType, - []byte(jsonPatch), + jsonPatch, metav1.PatchOptions{}, ) return err diff --git a/internal/dao/patch.go b/internal/dao/patch.go index f158777d..769e160d 100644 --- a/internal/dao/patch.go +++ b/internal/dao/patch.go @@ -4,6 +4,14 @@ import ( "encoding/json" ) +type ImageSpec struct { + Index int + Name, DockerImage string + Init bool +} + +type ImageSpecs []ImageSpec + type JsonPatch struct { Spec Spec `json:"spec"` } @@ -29,40 +37,42 @@ type Element struct { } // Build a json patch string to update PodSpec images -func GetTemplateJsonPatch(containersPatch map[string]string, initContainersPatch map[string]string) (string, error) { +func GetTemplateJsonPatch(imageSpecs ImageSpecs) ([]byte, error) { jsonPatch := JsonPatch{ Spec: Spec{ - Template: getPatchPodSpec(containersPatch, initContainersPatch), + Template: getPatchPodSpec(imageSpecs), }, } - bytes, err := json.Marshal(jsonPatch) - return string(bytes), err + return json.Marshal(jsonPatch) } -func GetJsonPatch(containersPatch map[string]string, initContainersPatch map[string]string) (string, error) { - podSpec := getPatchPodSpec(containersPatch, initContainersPatch) - bytes, err := json.Marshal(podSpec) - return string(bytes), err +func GetJsonPatch(imageSpecs ImageSpecs) ([]byte, error) { + podSpec := getPatchPodSpec(imageSpecs) + return json.Marshal(podSpec) } -func getPatchPodSpec(containersPatch map[string]string, initContainersPatch map[string]string) PodSpec { - elementsOrders, elements := extractElements(containersPatch) - initElementsOrders, initElements := extractElements(initContainersPatch) +func getPatchPodSpec(imageSpecs ImageSpecs) PodSpec { + initElementsOrders, initElements, elementsOrders, elements := extractElements(imageSpecs) podSpec := PodSpec{ Spec: ImagesSpec{ - SetElementOrderContainers: elementsOrders, - Containers: elements, SetElementOrderInitContainers: initElementsOrders, InitContainers: initElements, + SetElementOrderContainers: elementsOrders, + Containers: elements, }, } return podSpec } -func extractElements(containers map[string]string) (elementsOrders []Element, elements []Element) { - for name, image := range containers { - elementsOrders = append(elementsOrders, Element{Name: name}) - elements = append(elements, Element{Name: name, Image: image}) +func extractElements(imageSpecs ImageSpecs) (initElementsOrders []Element, initElements []Element, elementsOrders []Element, elements []Element) { + for _, spec := range imageSpecs { + if spec.Init { + initElementsOrders = append(initElementsOrders, Element{Name: spec.Name}) + initElements = append(initElements, Element{Name: spec.Name, Image: spec.DockerImage}) + } else { + elementsOrders = append(elementsOrders, Element{Name: spec.Name}) + elements = append(elements, Element{Name: spec.Name, Image: spec.DockerImage}) + } } - return elementsOrders, elements + return initElementsOrders, initElements, elementsOrders, elements } diff --git a/internal/dao/patch_test.go b/internal/dao/patch_test.go index 9086514a..439a3fc7 100644 --- a/internal/dao/patch_test.go +++ b/internal/dao/patch_test.go @@ -7,8 +7,7 @@ import ( func TestGetTemplateJsonPatch(t *testing.T) { type args struct { - containers map[string]string - initContainers map[string]string + imageSpecs ImageSpecs } tests := map[string]struct { args args @@ -17,29 +16,40 @@ func TestGetTemplateJsonPatch(t *testing.T) { }{ "simple": { args: args{ - initContainers: map[string]string{"init": "busybox:latest"}, - containers: map[string]string{"nginx": "nginx:latest"}, + imageSpecs: ImageSpecs{ + ImageSpec{ + Index: 0, + Name: "init", + DockerImage: "busybox:latest", + Init: true, + }, + ImageSpec{ + Index: 0, + Name: "nginx", + DockerImage: "nginx:latest", + Init: false, + }, + }, }, - want: `{"spec":{"template":{"spec":{"$setElementOrder/containers":[{"name":"nginx"}],"$setElementOrder/initContainers":[{"name":"init"}],"containers":[{"image":"nginx:latest","name":"nginx"}],"initContainers":[{"image":"busybox:latest","name":"init"}]}}}}`, + want: `{"spec":{"template":{"spec":{"$setElementOrder/initContainers":[{"name":"init"}],"$setElementOrder/containers":[{"name":"nginx"}],"initContainers":[{"image":"busybox:latest","name":"init"}],"containers":[{"image":"nginx:latest","name":"nginx"}]}}}}`, wantErr: false, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { - got, err := GetTemplateJsonPatch(tt.args.containers, tt.args.initContainers) + got, err := GetTemplateJsonPatch(tt.args.imageSpecs) if (err != nil) != tt.wantErr { t.Errorf("GetTemplateJsonPatch() error = %v, wantErr %v", err, tt.wantErr) return } - require.JSONEq(t, tt.want, got, "Json strings should be equal") + require.JSONEq(t, tt.want, string(got), "Json strings should be equal") }) } } func TestGetJsonPatch(t *testing.T) { type args struct { - containers map[string]string - initContainers map[string]string + imageSpecs ImageSpecs } tests := map[string]struct { args args @@ -48,21 +58,33 @@ func TestGetJsonPatch(t *testing.T) { }{ "simple": { args: args{ - initContainers: map[string]string{"init": "busybox:latest"}, - containers: map[string]string{"nginx": "nginx:latest"}, + imageSpecs: ImageSpecs{ + ImageSpec{ + Index: 0, + Name: "init", + DockerImage: "busybox:latest", + Init: true, + }, + ImageSpec{ + Index: 0, + Name: "nginx", + DockerImage: "nginx:latest", + Init: false, + }, + }, }, - want: `{"spec":{"$setElementOrder/containers":[{"name":"nginx"}],"$setElementOrder/initContainers":[{"name":"init"}],"containers":[{"image":"nginx:latest","name":"nginx"}],"initContainers":[{"image":"busybox:latest","name":"init"}]}}`, + want: `{"spec":{"$setElementOrder/initContainers":[{"name":"init"}],"initContainers":[{"image":"busybox:latest","name":"init"}],"$setElementOrder/containers":[{"name":"nginx"}],"containers":[{"image":"nginx:latest","name":"nginx"}]}}`, wantErr: false, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { - got, err := GetJsonPatch(tt.args.containers, tt.args.initContainers) + got, err := GetJsonPatch(tt.args.imageSpecs) if (err != nil) != tt.wantErr { t.Errorf("GetTemplateJsonPatch() error = %v, wantErr %v", err, tt.wantErr) return } - require.JSONEq(t, tt.want, got, "Json strings should be equal") + require.JSONEq(t, tt.want, string(got), "Json strings should be equal") }) } } diff --git a/internal/dao/pod.go b/internal/dao/pod.go index cce528a5..13169fce 100644 --- a/internal/dao/pod.go +++ b/internal/dao/pod.go @@ -467,7 +467,7 @@ func (p *Pod) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (p *Pod) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { +func (p *Pod) SetImages(ctx context.Context, path string, imageSpecs ImageSpecs) error { ns, n := client.Namespaced(path) auth, err := p.Client().CanI(ns, "v1/pod", []string{client.PatchVerb}) if err != nil { @@ -480,9 +480,9 @@ func (p *Pod) SetImages(ctx context.Context, path string, containersPatch map[st if err != nil { return err } - return fmt.Errorf("Unable to set image. This pod is managed by %s. Please set the image on the controller", *manager) + return fmt.Errorf("Unable to set image. This pod is managed by %s. Please set the image on the controller", manager) } - jsonPatch, err := GetJsonPatch(containersPatch, initContainersPatch) + jsonPatch, err := GetJsonPatch(imageSpecs) if err != nil { return err } @@ -494,26 +494,20 @@ func (p *Pod) SetImages(ctx context.Context, path string, containersPatch map[st ctx, n, types.StrategicMergePatchType, - []byte(jsonPatch), + jsonPatch, metav1.PatchOptions{}, ) return err } -func (p *Pod) isControlled(path string) (*string, bool, error) { +func (p *Pod) isControlled(path string) (string, bool, error) { pod, err := p.GetInstance(path) if err != nil { - return nil, false, err + return "", false, err } references := pod.GetObjectMeta().GetOwnerReferences() if len(references) != 0 && *references[0].Controller == true { - return getController(references[0]), true, nil + return fmt.Sprintf("%s/%s", references[0].Kind, references[0].Name), true, nil } - return nil, false, nil -} - -func getController(reference metav1.OwnerReference) *string { - var result string - result = fmt.Sprintf("%s/%s", reference.Kind, reference.Name) - return &result + return "", false, nil } diff --git a/internal/dao/sts.go b/internal/dao/sts.go index 80f36bf1..8e7498ab 100644 --- a/internal/dao/sts.go +++ b/internal/dao/sts.go @@ -231,7 +231,7 @@ func (s *StatefulSet) GetPodSpec(path string) (*v1.PodSpec, error) { return &podSpec, nil } -func (s *StatefulSet) SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { +func (s *StatefulSet) SetImages(ctx context.Context, path string, imageSpecs ImageSpecs) error { ns, n := client.Namespaced(path) auth, err := s.Client().CanI(ns, "apps/v1/statefulset", []string{client.PatchVerb}) if err != nil { @@ -240,7 +240,7 @@ func (s *StatefulSet) SetImages(ctx context.Context, path string, containersPatc if !auth { return fmt.Errorf("user is not authorized to patch a statefulset") } - jsonPatch, err := GetTemplateJsonPatch(containersPatch, initContainersPatch) + jsonPatch, err := GetTemplateJsonPatch(imageSpecs) if err != nil { return err } @@ -252,7 +252,7 @@ func (s *StatefulSet) SetImages(ctx context.Context, path string, containersPatc ctx, n, types.StrategicMergePatchType, - []byte(jsonPatch), + jsonPatch, metav1.PatchOptions{}, ) return err diff --git a/internal/dao/types.go b/internal/dao/types.go index d395f610..0f858411 100644 --- a/internal/dao/types.go +++ b/internal/dao/types.go @@ -152,5 +152,5 @@ type ContainsPodSpec interface { GetPodSpec(path string) (*v1.PodSpec, error) // Set Images for a resource - SetImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error + SetImages(ctx context.Context, path string, imageSpecs ImageSpecs) error } diff --git a/internal/view/set_image_extender.go b/internal/view/set_image_extender.go index 5970c32e..68561acd 100644 --- a/internal/view/set_image_extender.go +++ b/internal/view/set_image_extender.go @@ -9,25 +9,40 @@ import ( "github.com/gdamore/tcell" "github.com/rs/zerolog/log" corev1 "k8s.io/api/core/v1" + "strings" ) +const setImageKey = "setImage" + // SetImageExtender adds set image extensions type SetImageExtender struct { ResourceViewer } -type ContainerType string - -type ContainerImage struct { - ContainerType ContainerType - Image string +type imageFormSpec struct { + name, dockerImage, newDockerImage string + init bool } -const ( - setImageKey = "setImage" - runningContainer = ContainerType("Container") - initContainer = ContainerType("InitContainer") -) +func (m *imageFormSpec) modified() bool { + var newDockerImage = strings.TrimSpace(m.newDockerImage) + return newDockerImage != "" && m.dockerImage != newDockerImage +} + +func (m *imageFormSpec) imageSpec() dao.ImageSpec { + var ret = dao.ImageSpec{ + Name: m.name, + Init: m.init, + } + + if m.modified() { + ret.DockerImage = strings.TrimSpace(m.newDockerImage) + } else { + ret.DockerImage = m.dockerImage + } + + return ret +} func NewSetImageExtender(r ResourceViewer) ResourceViewer { s := SetImageExtender{ResourceViewer: r} @@ -68,26 +83,40 @@ func (s *SetImageExtender) showSetImageDialog(path string) { func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { f := s.makeStyledForm() podSpec, err := s.getPodSpec(sel) - containers, initContainers := getImages(podSpec) - containersResult := make(map[string]string) - initContainersResult := make(map[string]string) if err != nil { s.App().Flash().Err(err) return nil } - addInputField(f, &containers, &containersResult) - addInputField(f, &initContainers, &initContainersResult) + var formContainerLines []imageFormSpec + for _, spec := range podSpec.InitContainers { + formContainerLines = append(formContainerLines, imageFormSpec{init: true, name: spec.Name, dockerImage: spec.Image}) + } + for _, spec := range podSpec.Containers { + formContainerLines = append(formContainerLines, imageFormSpec{init: false, name: spec.Name, dockerImage: spec.Image}) + } + for _, ctn := range formContainerLines { + ctnCopy := ctn + f.AddInputField(ctn.name, ctn.dockerImage, 0, nil, func(changed string) { + ctnCopy.newDockerImage = changed + }) + } f.AddButton("OK", func() { defer s.dismissDialog() - if err != nil { s.App().Flash().Err(err) return } ctx, cancel := context.WithTimeout(context.Background(), s.App().Conn().Config().CallTimeout()) defer cancel() - if err := s.setImages(ctx, sel, containersResult, initContainersResult); err != nil { + var imageSpecsModified dao.ImageSpecs + for _, v := range formContainerLines { + if v.modified() { + imageSpecsModified = append(imageSpecsModified, v.imageSpec()) + } + } + + if err := s.setImages(ctx, sel, imageSpecsModified); err != nil { log.Error().Err(err).Msgf("PodSpec %s image update failed", sel) s.App().Flash().Err(err) } else { @@ -97,34 +126,9 @@ func (s *SetImageExtender) makeSetImageForm(sel string) *tview.Form { f.AddButton("Cancel", func() { s.dismissDialog() }) - return f } -func addInputField(f *tview.Form, containers *map[string]string, containersResult *map[string]string) { - for name, image := range *containers { - f.AddInputField(name, image, 0, nil, func(changed string) { - if changed == image { - delete(*containersResult, name) - } else { - (*containersResult)[name] = changed - } - }) - } -} - -func getImages(podSpec *corev1.PodSpec) (map[string]string, map[string]string) { - containers := make(map[string]string) - initContainers := make(map[string]string) - for _, c := range podSpec.Containers { - containers[c.Name] = c.Image - } - for _, c := range podSpec.InitContainers { - initContainers[c.Name] = c.Image - } - return containers, initContainers -} - func (s *SetImageExtender) dismissDialog() { s.App().Content.RemovePage(setImageKey) } @@ -149,11 +153,10 @@ func (s *SetImageExtender) getPodSpec(path string) (*corev1.PodSpec, error) { if !ok { return nil, fmt.Errorf("expecting a resourceWPodSpec resource for %q", s.GVR()) } - podSpec, err := resourceWPodSpec.GetPodSpec(path) - return podSpec, nil + return resourceWPodSpec.GetPodSpec(path) } -func (s *SetImageExtender) setImages(ctx context.Context, path string, containersPatch map[string]string, initContainersPatch map[string]string) error { +func (s *SetImageExtender) setImages(ctx context.Context, path string, imageSpecs dao.ImageSpecs) error { res, err := dao.AccessorFor(s.App().factory, s.GVR()) if err != nil { return err @@ -164,5 +167,5 @@ func (s *SetImageExtender) setImages(ctx context.Context, path string, container return fmt.Errorf("expecting a scalable resource for %q", s.GVR()) } - return resourceWPodSpec.SetImages(ctx, path, containersPatch, initContainersPatch) + return resourceWPodSpec.SetImages(ctx, path, imageSpecs) }