fix: replace unordered map by ImageSpec array

fix comments issues

Co-authored-by: Sylvain Dumas <sylvain.dumas@ext.leroymerlin.fr>
mine
Antoine Meausoone 2020-10-15 22:50:11 +02:00
parent 6f9f9ccea5
commit 27b1bba671
8 changed files with 131 additions and 102 deletions

View File

@ -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

View File

@ -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

View File

@ -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
}

View File

@ -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")
})
}
}

View File

@ -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
}

View File

@ -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

View File

@ -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
}

View File

@ -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)
}