Skip to content

Commit 4bdb51c

Browse files
informalictajanikow
informalict
authored and
ajanikow
committed
Discard alpha and beta phrases from annotations (#492)
1 parent 30c09d0 commit 4bdb51c

File tree

4 files changed

+154
-80
lines changed

4 files changed

+154
-80
lines changed

‎pkg/util/k8sutil/constants.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,8 @@ const (
3030
ArangoExporterPort = 9101
3131

3232
// K8s constants
33-
ClusterIPNone = "None"
34-
TolerateUnreadyEndpointsAnnotation = "service.alpha.kubernetes.io/tolerate-unready-endpoints"
35-
TopologyKeyHostname = "kubernetes.io/hostname"
33+
ClusterIPNone = "None"
34+
TopologyKeyHostname = "kubernetes.io/hostname"
3635

3736
// Internal constants
3837
ImageIDAndVersionRole = "id" // Role use by identification pods

‎pkg/util/k8sutil/services.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,14 +188,9 @@ func createService(svcs ServiceInterface, svcName, deploymentName, ns, clusterIP
188188
labels := LabelsForDeployment(deploymentName, role)
189189
svc := &v1.Service{
190190
ObjectMeta: metav1.ObjectMeta{
191-
Name: svcName,
192-
Labels: labels,
193-
Annotations: map[string]string{
194-
// This annotation is deprecated, PublishNotReadyAddresses is
195-
// used instead. We leave the annotation in for a while.
196-
// See https://github.com/kubernetes/kubernetes/pull/49061
197-
TolerateUnreadyEndpointsAnnotation: strconv.FormatBool(publishNotReadyAddresses),
198-
},
191+
Name: svcName,
192+
Labels: labels,
193+
Annotations: map[string]string{},
199194
},
200195
Spec: v1.ServiceSpec{
201196
Type: serviceType,

‎pkg/util/k8sutil/storageclass.go‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,22 @@ import (
2626
"strconv"
2727
"time"
2828

29-
"k8s.io/api/storage/v1"
29+
"github.com/arangodb/kube-arangodb/pkg/util/retry"
30+
v1 "k8s.io/api/storage/v1"
3031
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3132
storagev1 "k8s.io/client-go/kubernetes/typed/storage/v1"
32-
33-
"github.com/arangodb/kube-arangodb/pkg/util/retry"
3433
)
3534

3635
var (
37-
annStorageClassIsDefault = []string{
38-
// Make sure first entry is the one we'll put in
39-
"storageclass.kubernetes.io/is-default-class",
40-
"storageclass.beta.kubernetes.io/is-default-class",
41-
}
36+
annStorageClassIsDefault = "storageclass.kubernetes.io/is-default-class"
4237
)
4338

4439
// StorageClassIsDefault returns true if the given storage class is marked default,
4540
// false otherwise.
4641
func StorageClassIsDefault(sc *v1.StorageClass) bool {
47-
for _, key := range annStorageClassIsDefault {
48-
if value, found := sc.GetObjectMeta().GetAnnotations()[key]; found {
49-
if boolValue, err := strconv.ParseBool(value); err == nil && boolValue {
50-
return true
51-
}
42+
if value, found := sc.GetObjectMeta().GetAnnotations()[annStorageClassIsDefault]; found {
43+
if boolValue, err := strconv.ParseBool(value); err == nil && boolValue {
44+
return true
5245
}
5346
}
5447
return false
@@ -70,11 +63,9 @@ func PatchStorageClassIsDefault(cli storagev1.StorageV1Interface, name string, i
7063
if ann == nil {
7164
ann = make(map[string]string)
7265
}
73-
for _, key := range annStorageClassIsDefault {
74-
delete(ann, key)
75-
}
76-
ann[annStorageClassIsDefault[0]] = strconv.FormatBool(isDefault)
66+
ann[annStorageClassIsDefault] = strconv.FormatBool(isDefault)
7767
current.SetAnnotations(ann)
68+
7869
// Save StorageClass
7970
if _, err := stcs.Update(current); IsConflict(err) {
8071
// StorageClass has been modified since we read it

‎pkg/util/k8sutil/storageclass_test.go‎

Lines changed: 141 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -23,78 +23,167 @@
2323
package k8sutil
2424

2525
import (
26+
"errors"
2627
"testing"
2728

28-
"k8s.io/api/storage/v1"
29+
"github.com/arangodb/kube-arangodb/pkg/util/retry"
30+
"github.com/stretchr/testify/require"
31+
"k8s.io/apimachinery/pkg/runtime"
32+
33+
"github.com/stretchr/testify/assert"
34+
v1 "k8s.io/api/storage/v1"
35+
er "k8s.io/apimachinery/pkg/api/errors"
2936
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
37+
"k8s.io/client-go/kubernetes/fake"
38+
k8stesting "k8s.io/client-go/testing"
3039
)
3140

32-
// StorageClassIsDefault returns true if the given storage class is marked default,
33-
// false otherwise.
3441
func TestStorageClassIsDefault(t *testing.T) {
35-
tests := []struct {
42+
testCases := []struct {
43+
Name string
3644
StorageClass v1.StorageClass
3745
IsDefault bool
3846
}{
39-
// final annotation
40-
{v1.StorageClass{
41-
ObjectMeta: metav1.ObjectMeta{
42-
Annotations: map[string]string{},
47+
{
48+
Name: "Storage class without annotations",
49+
StorageClass: v1.StorageClass{
50+
ObjectMeta: metav1.ObjectMeta{},
4351
},
44-
}, false},
45-
{v1.StorageClass{
46-
ObjectMeta: metav1.ObjectMeta{
47-
Annotations: map[string]string{
48-
annStorageClassIsDefault[0]: "false",
52+
IsDefault: false,
53+
},
54+
{
55+
Name: "Storage class with empty annotations",
56+
StorageClass: v1.StorageClass{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Annotations: map[string]string{},
4959
},
5060
},
51-
}, false},
52-
{v1.StorageClass{
53-
ObjectMeta: metav1.ObjectMeta{
54-
Annotations: map[string]string{
55-
annStorageClassIsDefault[0]: "foo",
61+
IsDefault: false,
62+
},
63+
{
64+
Name: "Storage class without default",
65+
StorageClass: v1.StorageClass{
66+
ObjectMeta: metav1.ObjectMeta{
67+
Annotations: map[string]string{
68+
annStorageClassIsDefault: "false",
69+
},
5670
},
5771
},
58-
}, false},
59-
{v1.StorageClass{
60-
ObjectMeta: metav1.ObjectMeta{
61-
Annotations: map[string]string{
62-
annStorageClassIsDefault[0]: "true",
72+
IsDefault: false,
73+
},
74+
{
75+
Name: "Storage class with invalid value in annotation",
76+
StorageClass: v1.StorageClass{
77+
ObjectMeta: metav1.ObjectMeta{
78+
Annotations: map[string]string{
79+
annStorageClassIsDefault: "foo",
80+
},
6381
},
6482
},
65-
}, true},
66-
// beta annotation
67-
{v1.StorageClass{
68-
ObjectMeta: metav1.ObjectMeta{
69-
Annotations: map[string]string{},
70-
},
71-
}, false},
72-
{v1.StorageClass{
73-
ObjectMeta: metav1.ObjectMeta{
74-
Annotations: map[string]string{
75-
annStorageClassIsDefault[1]: "false",
83+
IsDefault: false,
84+
},
85+
{
86+
Name: "Default storage class exits",
87+
StorageClass: v1.StorageClass{
88+
ObjectMeta: metav1.ObjectMeta{
89+
Annotations: map[string]string{
90+
annStorageClassIsDefault: "true",
91+
},
7692
},
7793
},
78-
}, false},
79-
{v1.StorageClass{
80-
ObjectMeta: metav1.ObjectMeta{
81-
Annotations: map[string]string{
82-
annStorageClassIsDefault[1]: "foo",
83-
},
94+
IsDefault: true,
95+
},
96+
}
97+
98+
for _, testCase := range testCases {
99+
t.Run(testCase.Name, func(t *testing.T) {
100+
result := StorageClassIsDefault(&testCase.StorageClass)
101+
assert.Equal(t, testCase.IsDefault, result, "StorageClassIsDefault failed. Expected %v, got %v for %#v",
102+
testCase.IsDefault, result, testCase.StorageClass)
103+
})
104+
}
105+
}
106+
107+
func TestPatchStorageClassIsDefault(t *testing.T) {
108+
// Arrange
109+
resourceName := "storageclasses"
110+
testCases := []struct {
111+
Name string
112+
StorageClassName string
113+
ExpectedErr error
114+
Reactor func(action k8stesting.Action) (handled bool, ret runtime.Object, err error)
115+
ReactorActionVerb string
116+
}{
117+
{
118+
Name: "Set storage class is set to default",
119+
StorageClassName: "test",
120+
},
121+
{
122+
Name: "Storage class does not exist",
123+
StorageClassName: "invalid",
124+
ExpectedErr: er.NewNotFound(v1.Resource(resourceName), "invalid"),
125+
},
126+
{
127+
Name: "Can not get storage class from kubernetes",
128+
StorageClassName: "test",
129+
Reactor: func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
130+
return true, nil, retry.Permanent(errors.New("test"))
84131
},
85-
}, false},
86-
{v1.StorageClass{
87-
ObjectMeta: metav1.ObjectMeta{
88-
Annotations: map[string]string{
89-
annStorageClassIsDefault[1]: "true",
90-
},
132+
ReactorActionVerb: "get",
133+
ExpectedErr: errors.New("test"),
134+
},
135+
{
136+
Name: "Can not update storage class",
137+
StorageClassName: "test",
138+
Reactor: func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
139+
return true, nil, errors.New("test")
91140
},
92-
}, true},
141+
ReactorActionVerb: "update",
142+
ExpectedErr: errors.New("test"),
143+
},
144+
{
145+
Name: "Can not update Storage class due to permanent conflict",
146+
StorageClassName: "test",
147+
Reactor: func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
148+
return true, nil,
149+
retry.Permanent(er.NewConflict(v1.Resource(resourceName), "test", nil))
150+
},
151+
ReactorActionVerb: "update",
152+
ExpectedErr: er.NewConflict(v1.Resource(resourceName), "test", nil),
153+
},
93154
}
94-
for _, test := range tests {
95-
result := StorageClassIsDefault(&test.StorageClass)
96-
if result != test.IsDefault {
97-
t.Errorf("StorageClassIsDefault failed. Expected %v, got %v for %#v", test.IsDefault, result, test.StorageClass)
98-
}
155+
156+
for _, testCase := range testCases {
157+
//nolint:scopelint
158+
t.Run(testCase.Name, func(t *testing.T) {
159+
// Arrange
160+
var err error
161+
162+
clientSet := fake.NewSimpleClientset()
163+
storageSet := clientSet.StorageV1()
164+
_, err = storageSet.StorageClasses().Create(&v1.StorageClass{
165+
TypeMeta: metav1.TypeMeta{},
166+
ObjectMeta: metav1.ObjectMeta{
167+
Name: "test",
168+
},
169+
})
170+
require.NoError(t, err)
171+
172+
if testCase.Reactor != nil {
173+
clientSet.PrependReactor(testCase.ReactorActionVerb, resourceName, testCase.Reactor)
174+
}
175+
176+
// Act
177+
err = PatchStorageClassIsDefault(storageSet, testCase.StorageClassName, true)
178+
179+
// Assert
180+
if testCase.ExpectedErr != nil {
181+
require.EqualError(t, err, testCase.ExpectedErr.Error())
182+
return
183+
}
184+
185+
assert.NoError(t, err)
186+
})
99187
}
188+
100189
}

0 commit comments

Comments
 (0)