Skip to content

Commit 4d5bca4

Browse files
mjudeikismjudeikis
authored
Support non-namespaces creates (#163)
* Support non-namespaced creates * add tests for non-namespace creates * pin GOLANGCI_LINT
1 parent a89be2e commit 4d5bca4

File tree

7 files changed

+142
-38
lines changed

7 files changed

+142
-38
lines changed

‎.github/workflows/golangci-lint.yaml‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,4 @@ jobs:
1313
go-version: 'stable'
1414
- uses: actions/checkout@v4
1515
- name: golangci-lint
16-
uses: golangci/golangci-lint-action@v6
17-
with:
18-
args: --timeout=5m
16+
run: make lint

‎Makefile‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,19 @@
11
export API_TAGS ?= ExternalClusterAPI,AuthTokenAPI,OperationsAPI,AutoscalerAPI
22
export SWAGGER_LOCATION ?= https://api.cast.ai/v1/spec/openapi.json
33

4+
GO_INSTALL = ./hack/go-install.sh
5+
6+
TOOLS_DIR=bin
7+
ROOT_DIR=$(abspath .)
8+
TOOLS_GOBIN_DIR := $(abspath $(TOOLS_DIR))
9+
10+
GOLANGCI_LINT_VER := v1.62.2
11+
GOLANGCI_LINT_BIN := golangci-lint
12+
GOLANGCI_LINT := $(TOOLS_GOBIN_DIR)/$(GOLANGCI_LINT_BIN)-$(GOLANGCI_LINT_VER)
13+
14+
$(GOLANGCI_LINT):
15+
GOBIN=$(TOOLS_GOBIN_DIR) $(GO_INSTALL) github.com/golangci/golangci-lint/cmd/golangci-lint $(GOLANGCI_LINT_BIN) $(GOLANGCI_LINT_VER)
16+
417
build:
518
CGO_ENABLED=0 GOOS=linux go build -ldflags "-s -w" -o bin/castai-cluster-controller-amd64 .
619
docker build -t us-docker.pkg.dev/castai-hub/library/cluster-controller:$(VERSION) .
@@ -10,12 +23,12 @@ push:
1023

1124
release: build push
1225

13-
lint:
14-
golangci-lint run ./...
26+
lint: $(GOLANGCI_LINT)
27+
$(GOLANGCI_LINT) run --timeout 20m ./...
1528
.PHONY: lint
1629

17-
fix:
18-
golangci-lint run --fix ./...
30+
fix: $(GOLANGCI_LINT)
31+
$(GOLANGCI_LINT) run --fix ./...
1932
.PHONY: fix
2033

2134
test:

‎hack/go-install.sh‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
#!/usr/bin/env bash
2+
3+
# Originally copied from
4+
# https://github.com/kubernetes-sigs/cluster-api-provider-gcp/blob/c26a68b23e9317323d5d37660fe9d29b3d2ff40c/scripts/go_install.sh
5+
6+
set -o errexit
7+
set -o nounset
8+
set -o pipefail
9+
10+
if [[ -z "${1:-}" ]]; then
11+
echo "must provide module as first parameter"
12+
exit 1
13+
fi
14+
15+
if [[ -z "${2:-}" ]]; then
16+
echo "must provide binary name as second parameter"
17+
exit 1
18+
fi
19+
20+
if [[ -z "${3:-}" ]]; then
21+
echo "must provide version as third parameter"
22+
exit 1
23+
fi
24+
25+
if [[ -z "${GOBIN:-}" ]]; then
26+
echo "GOBIN is not set. Must set GOBIN to install the bin in a specified directory."
27+
exit 1
28+
fi
29+
30+
mkdir -p "${GOBIN}"
31+
32+
tmp_dir=$(mktemp -d -t goinstall_XXXXXXXXXX)
33+
function clean {
34+
rm -rf "${tmp_dir}"
35+
}
36+
trap clean EXIT
37+
38+
rm "${GOBIN}/${2}"* > /dev/null 2>&1 || true
39+
40+
cd "${tmp_dir}"
41+
42+
# create a new module in the tmp directory
43+
go mod init fake/mod
44+
45+
# install the golang module specified as the first argument
46+
go install -tags kcptools "${1}@${3}"
47+
mv "${GOBIN}/${2}" "${GOBIN}/${2}-${3}"
48+
ln -sf "${GOBIN}/${2}-${3}" "${GOBIN}/${2}"

‎internal/actions/create_handler.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,6 @@ func (h *CreateHandler) Handle(ctx context.Context, action *castai.ClusterAction
4242
}
4343

4444
newObj := &unstructured.Unstructured{Object: req.Object}
45-
if newObj.GetNamespace() == "" {
46-
return fmt.Errorf("namespace not provided %w", errAction)
47-
}
4845

4946
log := h.log.WithFields(logrus.Fields{
5047
ActionIDLogField: action.ID,
@@ -53,21 +50,26 @@ func (h *CreateHandler) Handle(ctx context.Context, action *castai.ClusterAction
5350
"name": newObj.GetName(),
5451
})
5552

56-
r := h.client.Resource(schema.GroupVersionResource{
53+
gvkResource := h.client.Resource(schema.GroupVersionResource{
5754
Group: req.Group,
5855
Version: req.Version,
5956
Resource: req.Resource,
60-
}).Namespace(newObj.GetNamespace())
57+
})
58+
59+
var resource dynamic.ResourceInterface = gvkResource
60+
if newObj.GetNamespace() != "" {
61+
resource = gvkResource.Namespace(newObj.GetNamespace())
62+
}
6163

6264
log.Info("creating new resource")
63-
_, err := r.Create(ctx, newObj, metav1.CreateOptions{})
65+
_, err := resource.Create(ctx, newObj, metav1.CreateOptions{})
6466
if err != nil && !apierrors.IsAlreadyExists(err) {
6567
return fmt.Errorf("creating resource %v: %w", req.Resource, err)
6668
}
6769

6870
if apierrors.IsAlreadyExists(err) {
6971
log.Info("resource already exists, patching")
70-
obj, err := r.Get(ctx, newObj.GetName(), metav1.GetOptions{})
72+
obj, err := resource.Get(ctx, newObj.GetName(), metav1.GetOptions{})
7173
if err != nil {
7274
return fmt.Errorf("getting old resource: %w", err)
7375
}
@@ -106,7 +108,7 @@ func (h *CreateHandler) Handle(ctx context.Context, action *castai.ClusterAction
106108
}
107109

108110
log.Infof("patching resource: %s", patch)
109-
_, err = r.Patch(ctx, obj.GetName(), k8s_types.MergePatchType, patch, metav1.PatchOptions{})
111+
_, err = resource.Patch(ctx, obj.GetName(), k8s_types.MergePatchType, patch, metav1.PatchOptions{})
110112
if err != nil {
111113
return fmt.Errorf("patching resource %v: %w", obj.GetName(), err)
112114
}

‎internal/actions/create_handler_test.go‎

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
func Test_newCreateHandler(t *testing.T) {
2323
scheme := runtime.NewScheme()
2424
_ = appsv1.AddToScheme(scheme)
25+
_ = v1.AddToScheme(scheme)
2526
ctx := context.Background()
2627

2728
now := metav1.Time{Time: time.Date(2024, time.September, 1, 0, 0, 0, 0, time.Local)}
@@ -30,7 +31,7 @@ func Test_newCreateHandler(t *testing.T) {
3031
action *castai.ClusterAction
3132
convertFn func(i map[string]interface{}) client.Object
3233
err error
33-
want *appsv1.Deployment
34+
want runtime.Object
3435
}{
3536
"should return error when action is of a different type": {
3637
action: &castai.ClusterAction{
@@ -72,17 +73,17 @@ func Test_newCreateHandler(t *testing.T) {
7273
Version: appsv1.SchemeGroupVersion.Version,
7374
Resource: "deployments",
7475
},
75-
Object: getObj(t, newDeployment(func(d *appsv1.Deployment) {
76-
d.Labels = map[string]string{"changed": "true"}
76+
Object: getObj(t, newDeployment(func(d runtime.Object) {
77+
d.(*appsv1.Deployment).Labels = map[string]string{"changed": "true"}
7778
})),
7879
},
7980
},
80-
objs: []runtime.Object{newDeployment(func(d *appsv1.Deployment) {
81-
d.CreationTimestamp = now
81+
objs: []runtime.Object{newDeployment(func(d runtime.Object) {
82+
d.(*appsv1.Deployment).CreationTimestamp = now
8283
})},
83-
want: newDeployment(func(d *appsv1.Deployment) {
84-
d.CreationTimestamp = now
85-
d.Labels = map[string]string{"changed": "true"}
84+
want: newDeployment(func(d runtime.Object) {
85+
d.(*appsv1.Deployment).CreationTimestamp = now
86+
d.(*appsv1.Deployment).Labels = map[string]string{"changed": "true"}
8687
}),
8788
convertFn: func(i map[string]interface{}) client.Object {
8889
out := &appsv1.Deployment{}
@@ -98,24 +99,42 @@ func Test_newCreateHandler(t *testing.T) {
9899
Version: appsv1.SchemeGroupVersion.Version,
99100
Resource: "deployments",
100101
},
101-
Object: getObj(t, newDeployment(func(d *appsv1.Deployment) {
102+
Object: getObj(t, newDeployment(func(d runtime.Object) {
102103
})),
103104
},
104105
},
105-
objs: []runtime.Object{newDeployment(func(d *appsv1.Deployment) {
106-
d.CreationTimestamp = now
107-
d.Finalizers = []string{"autoscaling.cast.ai/recommendation"}
106+
objs: []runtime.Object{newDeployment(func(d runtime.Object) {
107+
d.(*appsv1.Deployment).CreationTimestamp = now
108+
d.(*appsv1.Deployment).Finalizers = []string{"autoscaling.cast.ai/recommendation"}
108109
})},
109-
want: newDeployment(func(d *appsv1.Deployment) {
110-
d.CreationTimestamp = now
111-
d.Finalizers = []string{"autoscaling.cast.ai/recommendation"}
110+
want: newDeployment(func(d runtime.Object) {
111+
d.(*appsv1.Deployment).CreationTimestamp = now
112+
d.(*appsv1.Deployment).Finalizers = []string{"autoscaling.cast.ai/recommendation"}
112113
}),
113114
convertFn: func(i map[string]interface{}) client.Object {
114115
out := &appsv1.Deployment{}
115116
_ = runtime.DefaultUnstructuredConverter.FromUnstructured(i, out)
116117
return out
117118
},
118119
},
120+
"should create new namespace": {
121+
action: &castai.ClusterAction{
122+
ActionCreate: &castai.ActionCreate{
123+
GroupVersionResource: castai.GroupVersionResource{
124+
Group: v1.SchemeGroupVersion.Group,
125+
Version: v1.SchemeGroupVersion.Version,
126+
Resource: "namespaces",
127+
},
128+
Object: getObj(t, newNamespace()),
129+
},
130+
},
131+
want: newNamespace(),
132+
convertFn: func(i map[string]interface{}) client.Object {
133+
out := &v1.Namespace{}
134+
_ = runtime.DefaultUnstructuredConverter.FromUnstructured(i, out)
135+
return out
136+
},
137+
},
119138
}
120139

121140
for name, test := range tests {
@@ -156,8 +175,8 @@ func getObj(t *testing.T, obj runtime.Object) map[string]interface{} {
156175
return unstructured
157176
}
158177

159-
func newDeployment(opts ...func(d *appsv1.Deployment)) *appsv1.Deployment {
160-
out := &appsv1.Deployment{
178+
func newDeployment(opts ...func(d runtime.Object)) runtime.Object {
179+
out := appsv1.Deployment{
161180
TypeMeta: metav1.TypeMeta{
162181
Kind: "Deployment",
163182
APIVersion: "apps/v1",
@@ -170,8 +189,26 @@ func newDeployment(opts ...func(d *appsv1.Deployment)) *appsv1.Deployment {
170189
Template: v1.PodTemplateSpec{},
171190
},
172191
}
192+
var obj runtime.Object = &out
193+
for _, opt := range opts {
194+
opt(obj)
195+
}
196+
return obj
197+
}
198+
199+
func newNamespace(opts ...func(d runtime.Object)) runtime.Object {
200+
out := v1.Namespace{
201+
TypeMeta: metav1.TypeMeta{
202+
Kind: "Namespace",
203+
APIVersion: "v1",
204+
},
205+
ObjectMeta: metav1.ObjectMeta{
206+
Name: "bob-namespace",
207+
},
208+
}
209+
var obj runtime.Object = &out
173210
for _, opt := range opts {
174-
opt(out)
211+
opt(obj)
175212
}
176-
return out
213+
return obj
177214
}

‎internal/actions/delete_handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (h *DeleteHandler) Handle(ctx context.Context, action *castai.ClusterAction
4848
})
4949

5050
var res dynamic.ResourceInterface = r
51-
if req.ID.Namespace != nil {
51+
if req.ID.Namespace != nil && *req.ID.Namespace != "" {
5252
res = r.Namespace(*req.ID.Namespace)
5353
}
5454

‎internal/actions/delete_handler_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ func Test_newDeleteHandler(t *testing.T) {
5050
},
5151
},
5252
objs: []runtime.Object{
53-
newDeployment(func(d *appsv1.Deployment) { d.SetName("nginx-1") }),
53+
newDeployment(func(d runtime.Object) {
54+
d.(*appsv1.Deployment).SetName("nginx-1")
55+
}),
5456
},
5557
want: 1,
5658
},
@@ -70,8 +72,12 @@ func Test_newDeleteHandler(t *testing.T) {
7072
},
7173
objs: []runtime.Object{
7274
newDeployment(),
73-
newDeployment(func(d *appsv1.Deployment) { d.SetName("nginx-1") }),
74-
newDeployment(func(d *appsv1.Deployment) { d.SetName("nginx-2") }),
75+
newDeployment(func(d runtime.Object) {
76+
d.(*appsv1.Deployment).SetName("nginx-1")
77+
}),
78+
newDeployment(func(d runtime.Object) {
79+
d.(*appsv1.Deployment).SetName("nginx-2")
80+
}),
7581
},
7682
want: 2,
7783
},

0 commit comments

Comments
 (0)