From c2d87bd1d8500e269a3436d9600fe7aa5cc805db Mon Sep 17 00:00:00 2001 From: Tomasz Mielech Date: Thu, 20 Jan 2022 15:07:37 +0100 Subject: [PATCH 1/4] Fix panic when fetching image ID from empty statuses --- cmd/main.go | 4 +++- pkg/deployment/images.go | 6 +++--- pkg/util/k8sutil/images.go | 27 ++++++++++++--------------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 1cc535e70..74f7c9ff6 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -442,7 +442,9 @@ func getMyPodInfo(kubecli kubernetes.Interface, namespace, name string) (string, return maskAny(err) } sa = pod.Spec.ServiceAccountName - image = k8sutil.GetArangoDBImageIDFromPod(pod) + if image, err = k8sutil.GetArangoDBImageIDFromPod(pod); err != nil { + return errors.Wrap(err, "failed to get image ID from pod") + } if image == "" { // Fallback in case we don't know the id. image = pod.Spec.Containers[0].Image diff --git a/pkg/deployment/images.go b/pkg/deployment/images.go index 8e3973a29..601d38c82 100644 --- a/pkg/deployment/images.go +++ b/pkg/deployment/images.go @@ -160,11 +160,11 @@ func (ib *imagesBuilder) fetchArangoDBImageIDAndVersion(ctx context.Context, cac return true, nil } - if len(pod.Status.ContainerStatuses) == 0 { - log.Warn().Msg("Empty list of ContainerStatuses") + imageID, err := k8sutil.GetArangoDBImageIDFromPod(pod) + if err != nil { + log.Warn().Err(err).Msg("failed to get image ID from pod") return true, nil } - imageID := k8sutil.GetArangoDBImageIDFromPod(pod) if imageID == "" { // Fall back to specified image imageID = image diff --git a/pkg/util/k8sutil/images.go b/pkg/util/k8sutil/images.go index a90ad908c..39c668b48 100644 --- a/pkg/util/k8sutil/images.go +++ b/pkg/util/k8sutil/images.go @@ -24,6 +24,8 @@ import ( "strings" corev1 "k8s.io/api/core/v1" + + "github.com/arangodb/kube-arangodb/pkg/util/errors" ) const ( @@ -40,7 +42,15 @@ func ConvertImageID2Image(imageID string) string { } // GetArangoDBImageIDFromPod returns the ArangoDB specific image from a pod -func GetArangoDBImageIDFromPod(pod *corev1.Pod) string { +func GetArangoDBImageIDFromPod(pod *corev1.Pod) (string, error) { + if pod == nil { + return "", errors.New("failed to get container statuses from nil pod") + } + + if len(pod.Status.ContainerStatuses) == 0 { + return "", errors.New("empty list of ContainerStatuses") + } + rawImageID := pod.Status.ContainerStatuses[0].ImageID if len(pod.Status.ContainerStatuses) > 1 { for _, containerStatus := range pod.Status.ContainerStatuses { @@ -49,19 +59,6 @@ func GetArangoDBImageIDFromPod(pod *corev1.Pod) string { } } } - return ConvertImageID2Image(rawImageID) -} - -// GetArangoDBContainerFromPod returns the ArangoDB container from a pod -func GetArangoDBContainerFromPod(pod *corev1.Pod) corev1.Container { - arangoc := pod.Spec.Containers[0] - if len(pod.Status.ContainerStatuses) > 1 { - for _, container := range pod.Spec.Containers { - if strings.Contains(container.Name, "server") { - arangoc = container - } - } - } - return arangoc + return ConvertImageID2Image(rawImageID), nil } From 1c6f81640176837790235f2c1b7d058d701909cc Mon Sep 17 00:00:00 2001 From: Tomasz Mielech Date: Thu, 20 Jan 2022 15:21:33 +0100 Subject: [PATCH 2/4] add unit tests --- pkg/util/k8sutil/images_test.go | 74 +++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 pkg/util/k8sutil/images_test.go diff --git a/pkg/util/k8sutil/images_test.go b/pkg/util/k8sutil/images_test.go new file mode 100644 index 000000000..0123730d2 --- /dev/null +++ b/pkg/util/k8sutil/images_test.go @@ -0,0 +1,74 @@ +package k8sutil + +import ( + "github.com/arangodb/kube-arangodb/pkg/util/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "testing" +) + +func TestGetArangoDBImageIDFromPod(t *testing.T) { + type args struct { + pod *corev1.Pod + } + tests := map[string]struct { + args args + want string + wantErr error + }{ + "pid is nil": { + wantErr: errors.New("failed to get container statuses from nil pod"), + }, + "container statuses list is empty": { + args: args{ + pod: &corev1.Pod{}, + }, + wantErr: errors.New("empty list of ContainerStatuses"), + }, + "image ID from the only container": { + args: args{ + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + ImageID: dockerPullableImageIDPrefix + "test", + }, + }, + }, + }, + }, + want: "test", + }, + "image ID from two containers": { + args: args{ + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + ImageID: dockerPullableImageIDPrefix + "test_arango", + }, + { + ImageID: dockerPullableImageIDPrefix + "test1_arango", + }, + }, + }, + }, + }, + want: "test1_arango", + }, + } + + for testName, testCase := range tests { + t.Run(testName, func(t *testing.T) { + got, err := GetArangoDBImageIDFromPod(testCase.args.pod) + if testCase.wantErr != nil { + require.EqualError(t, err, testCase.wantErr.Error()) + return + } + + require.NoError(t, err) + assert.Equalf(t, testCase.want, got, "image ID is not as expected") + }) + } +} From 86b8fe33208a812992a83e6777cef7c655ed34ad Mon Sep 17 00:00:00 2001 From: Tomasz Mielech Date: Thu, 20 Jan 2022 16:34:30 +0100 Subject: [PATCH 3/4] fix license disclaimer --- pkg/util/k8sutil/images_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/util/k8sutil/images_test.go b/pkg/util/k8sutil/images_test.go index 0123730d2..d7a9e4c5e 100644 --- a/pkg/util/k8sutil/images_test.go +++ b/pkg/util/k8sutil/images_test.go @@ -1,3 +1,23 @@ +// +// DISCLAIMER +// +// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// + package k8sutil import ( From 39c6cd250796d87e0e1a440e2ebd7ad96c917fbf Mon Sep 17 00:00:00 2001 From: Tomasz Mielech Date: Thu, 20 Jan 2022 20:16:05 +0100 Subject: [PATCH 4/4] fix imports' order --- pkg/util/k8sutil/images_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/util/k8sutil/images_test.go b/pkg/util/k8sutil/images_test.go index d7a9e4c5e..56662e13a 100644 --- a/pkg/util/k8sutil/images_test.go +++ b/pkg/util/k8sutil/images_test.go @@ -21,11 +21,12 @@ package k8sutil import ( + "testing" + "github.com/arangodb/kube-arangodb/pkg/util/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - "testing" ) func TestGetArangoDBImageIDFromPod(t *testing.T) {