Skip to content

Commit

Permalink
[Bugfix] Fix Image Error Propagation (#1603)
Browse files Browse the repository at this point in the history
  • Loading branch information
ajanikow authored Feb 29, 2024
1 parent 3e0f90f commit 25570fa
Show file tree
Hide file tree
Showing 4 changed files with 300 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- (Bugfix) Fix Image Discovery
- (Bugfix) Fix Resources Copy mechanism to prevent invalid pod creation
- (Bugfix) Wait for ImageStatus in ImageDiscover
- (Bugfix) Fix Image Error Propagation

## [1.2.38](https://github.com/arangodb/kube-arangodb/tree/1.2.38) (2024-02-22)
- (Feature) Extract GRPC Server
Expand Down
4 changes: 2 additions & 2 deletions cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ func getMyPodInfoWrap(kubecli kubernetes.Interface, namespace, name string, imag
}
sa = pod.Spec.ServiceAccountName
if i, ok := imageFunc(pod); !ok {
return errors.Wrap(err, "failed to get image ID from pod")
return errors.Errorf("failed to get image ID from pod")
} else {
image = i
}
Expand All @@ -633,7 +633,7 @@ func getMyImageInfoFunc(status bool) func(pod *core.Pod) (string, bool) {
if status {
return k8sutil.GetArangoDBImageIDFromContainerStatuses(pod.Status.ContainerStatuses, shared.ServerContainerName, shared.OperatorContainerName, constants.MyContainerNameEnv.GetOrDefault(shared.OperatorContainerName))
}
return k8sutil.GetArangoDBImageIDFromContainers(pod.Spec.Containers, shared.ServerContainerName, shared.OperatorContainerName, constants.MyContainerNameEnv.GetOrDefault(shared.OperatorContainerName))
return k8sutil.GetArangoDBImageFromContainers(pod.Spec.Containers, shared.ServerContainerName, shared.OperatorContainerName, constants.MyContainerNameEnv.GetOrDefault(shared.OperatorContainerName))
}
}

Expand Down
294 changes: 294 additions & 0 deletions cmd/cmd_pod_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,294 @@
//
// DISCLAIMER
//
// Copyright 2024 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 cmd

import (
"testing"
"time"

"github.com/stretchr/testify/require"
core "k8s.io/api/core/v1"
meta "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/arangodb/kube-arangodb/pkg/util"
"github.com/arangodb/kube-arangodb/pkg/util/kclient"
"github.com/arangodb/kube-arangodb/pkg/util/tests"
)

func Test_PodDiscovery(t *testing.T) {
operatorImageDiscovery.timeout = time.Millisecond

type testCase struct {
Name string

Pod core.Pod

Image, ServiceAccount string

Valid bool

DefaultStatusDiscovery *bool
}

var testCases = []testCase{
{
Name: "Empty pod",
Valid: false,
},
{
Name: "Not allowed containers",
Valid: false,
Pod: core.Pod{
ObjectMeta: meta.ObjectMeta{
Name: "operator",
Namespace: tests.FakeNamespace,
},
Spec: core.PodSpec{
Containers: []core.Container{
{
Name: "unknown",
Image: "image1",
},
},
},
Status: core.PodStatus{
ContainerStatuses: []core.ContainerStatus{
{
Name: "unknown",
Image: "image1",
ImageID: "image1",
},
},
},
},
},
{
Name: "Allowed Status & Spec",
Valid: true,
Image: "image1",
ServiceAccount: "sa",
Pod: core.Pod{
ObjectMeta: meta.ObjectMeta{
Name: "operator",
Namespace: tests.FakeNamespace,
},
Spec: core.PodSpec{
ServiceAccountName: "sa",
Containers: []core.Container{
{
Name: "operator",
Image: "image1",
},
},
},
Status: core.PodStatus{
ContainerStatuses: []core.ContainerStatus{
{
Name: "operator",
Image: "image1",
ImageID: "image1",
},
},
},
},
},
{
Name: "Allowed Status & Spec",
Valid: true,
Image: "imageStatusID1",
ServiceAccount: "sa",
Pod: core.Pod{
ObjectMeta: meta.ObjectMeta{
Name: "operator",
Namespace: tests.FakeNamespace,
},
Spec: core.PodSpec{
ServiceAccountName: "sa",
Containers: []core.Container{
{
Name: "operator",
Image: "imageSpec1",
},
},
},
Status: core.PodStatus{
ContainerStatuses: []core.ContainerStatus{
{
Name: "operator",
Image: "imageStatus1",
ImageID: "imageStatusID1",
},
},
},
},
},
{
Name: "Allowed Status & Spec - From Spec",
Valid: true,
Image: "imageSpec1",
ServiceAccount: "sa",
DefaultStatusDiscovery: util.NewType(false),
Pod: core.Pod{
ObjectMeta: meta.ObjectMeta{
Name: "operator",
Namespace: tests.FakeNamespace,
},
Spec: core.PodSpec{
ServiceAccountName: "sa",
Containers: []core.Container{
{
Name: "operator",
Image: "imageSpec1",
},
},
},
Status: core.PodStatus{
ContainerStatuses: []core.ContainerStatus{
{
Name: "operator",
Image: "imageStatus1",
ImageID: "imageStatusID1",
},
},
},
},
},
{
Name: "Allowed Spec",
Valid: true,
Image: "imageSpec1",
ServiceAccount: "sa",
Pod: core.Pod{
ObjectMeta: meta.ObjectMeta{
Name: "operator",
Namespace: tests.FakeNamespace,
},
Spec: core.PodSpec{
ServiceAccountName: "sa",
Containers: []core.Container{
{
Name: "operator",
Image: "imageSpec1",
},
},
},
},
},
{
Name: "Allowed Status & Spec - From Second Pod",
Valid: true,
Image: "imageStatusID2",
ServiceAccount: "sa",
Pod: core.Pod{
ObjectMeta: meta.ObjectMeta{
Name: "operator",
Namespace: tests.FakeNamespace,
},
Spec: core.PodSpec{
ServiceAccountName: "sa",
Containers: []core.Container{
{
Name: "test",
Image: "imageSpec1",
},
{
Name: "operator",
Image: "imageSpec2",
},
},
},
Status: core.PodStatus{
ContainerStatuses: []core.ContainerStatus{
{
Name: "test",
Image: "imageStatus1",
ImageID: "imageStatusID1",
},
{
Name: "operator",
Image: "imageStatus2",
ImageID: "imageStatusID2",
},
},
},
},
},
{
Name: "Allowed Status & Spec - From Second Pod Spec",
Valid: true,
Image: "imageSpec2",
ServiceAccount: "sa",
DefaultStatusDiscovery: util.NewType(false),
Pod: core.Pod{
ObjectMeta: meta.ObjectMeta{
Name: "operator",
Namespace: tests.FakeNamespace,
},
Spec: core.PodSpec{
ServiceAccountName: "sa",
Containers: []core.Container{
{
Name: "test",
Image: "imageSpec1",
},
{
Name: "operator",
Image: "imageSpec2",
},
},
},
Status: core.PodStatus{
ContainerStatuses: []core.ContainerStatus{
{
Name: "test",
Image: "imageStatus1",
ImageID: "imageStatusID1",
},
{
Name: "operator",
Image: "imageStatus2",
ImageID: "imageStatusID2",
},
},
},
},
},
}

for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
operatorImageDiscovery.defaultStatusDiscovery = util.TypeOrDefault(testCase.DefaultStatusDiscovery, true)

c := kclient.NewFakeClientBuilder().Add(&testCase.Pod).Client()
image, sa, err := getMyPodInfo(c.Kubernetes(), tests.FakeNamespace, "operator")

if !testCase.Valid {
require.Error(t, err)
require.Empty(t, image)
require.Empty(t, sa)
} else {
require.NoError(t, err)
require.Equal(t, testCase.Image, image)
require.Equal(t, testCase.ServiceAccount, sa)
}
})
}
}
6 changes: 3 additions & 3 deletions pkg/util/k8sutil/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func GetArangoDBImageIDFromPod(pod *core.Pod, names ...string) (string, error) {
return image, nil
}

if image, ok := GetArangoDBImageIDFromContainers(pod.Spec.Containers, names...); ok {
if image, ok := GetArangoDBImageFromContainers(pod.Spec.Containers, names...); ok {
return image, nil
}

Expand Down Expand Up @@ -89,8 +89,8 @@ func GetArangoDBImageIDFromContainerStatuses(containers []core.ContainerStatus,
return "", false
}

// GetArangoDBImageIDFromContainers returns the ArangoDB specific image from a container specs
func GetArangoDBImageIDFromContainers(containers []core.Container, names ...string) (string, bool) {
// GetArangoDBImageFromContainers returns the ArangoDB specific image from a container specs
func GetArangoDBImageFromContainers(containers []core.Container, names ...string) (string, bool) {
for _, name := range names {
if id := container.GetContainerIDByName(containers, name); id != -1 {
if image := containers[id].Image; image != "" {
Expand Down

0 comments on commit 25570fa

Please sign in to comment.