Skip to content

Commit 04bdf3b

Browse files
shyamradhakrishnanshyamradhakrishnan
authored
Fix OKE templates and add e2e tests (#195)
1 parent 3d13008 commit 04bdf3b

21 files changed

+470
-11
lines changed

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ generate-e2e-templates: $(KUSTOMIZE)
252252
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-remote-vcn-peering --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-remote-vcn-peering.yaml
253253
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-externally-managed-vcn --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-externally-managed-vcn.yaml
254254
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-machine-pool --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-machine-pool.yaml
255+
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-managed --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-managed.yaml
255256

256257
.PHONY: test-e2e-run
257258
test-e2e-run: generate-e2e-templates $(GINKGO) $(ENVSUBST) ## Run e2e tests

‎cloud/scope/managed_machine_pool.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (m *ManagedMachinePoolScope) FindNodePool(ctx context.Context) (*oke.NodePo
177177
reqList := oke.ListNodePoolsRequest{
178178
CompartmentId: common.String(m.OCIManagedCluster.Spec.CompartmentId),
179179
ClusterId: m.OCIManagedControlPlane.Spec.ID,
180-
Name: common.String(m.OCIManagedMachinePool.GetName()),
180+
Name: common.String(m.getNodePoolName()),
181181
Page: page,
182182
}
183183

@@ -199,6 +199,10 @@ func (m *ManagedMachinePoolScope) FindNodePool(ctx context.Context) (*oke.NodePo
199199
return nil, nil
200200
}
201201

202+
func (m *ManagedMachinePoolScope) getNodePoolName() string {
203+
return m.OCIManagedMachinePool.GetName()
204+
}
205+
202206
// CreateNodePool attempts to create a node pool
203207
func (m *ManagedMachinePoolScope) CreateNodePool(ctx context.Context) (*oke.NodePool, error) {
204208
m.Info("Creating Node Pool")
@@ -285,7 +289,7 @@ func (m *ManagedMachinePoolScope) CreateNodePool(ctx context.Context) (*oke.Node
285289
nodePoolDetails := oke.CreateNodePoolDetails{
286290
CompartmentId: common.String(m.OCIManagedCluster.Spec.CompartmentId),
287291
ClusterId: m.OCIManagedControlPlane.Spec.ID,
288-
Name: common.String(m.OCIManagedMachinePool.Name),
292+
Name: common.String(m.getNodePoolName()),
289293
KubernetesVersion: m.OCIManagedMachinePool.Spec.Version,
290294
NodeShape: common.String(m.OCIManagedMachinePool.Spec.NodeShape),
291295
NodeShapeConfig: &nodeShapeConfig,
@@ -506,7 +510,7 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
506510
}
507511
actual := m.getSpecFromAPIObject(pool)
508512
if !reflect.DeepEqual(spec, actual) ||
509-
m.OCIManagedMachinePool.Name != *pool.Name || nodePoolSizeUpdateRequired {
513+
m.getNodePoolName() != *pool.Name || nodePoolSizeUpdateRequired {
510514
m.Logger.Info("Updating node pool")
511515
// printing json specs will help debug problems when there are spurious/unwanted updates
512516
jsonSpec, err := json.Marshal(*spec)
@@ -574,7 +578,7 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
574578
}
575579
}
576580
nodePoolDetails := oke.UpdateNodePoolDetails{
577-
Name: common.String(m.OCIManagedMachinePool.Name),
581+
Name: common.String(m.getNodePoolName()),
578582
KubernetesVersion: m.OCIManagedMachinePool.Spec.Version,
579583
NodeShape: common.String(m.OCIManagedMachinePool.Spec.NodeShape),
580584
NodeShapeConfig: &nodeShapeConfig,

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedcontrolplanes.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ spec:
180180
type: boolean
181181
ready:
182182
type: boolean
183+
version:
184+
description: Version represents the current Kubernetes version for
185+
the control plane.
186+
type: string
183187
type: object
184188
type: object
185189
served: true

‎exp/api/v1beta1/ocimanagedcontrolplane_types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ type OCIManagedControlPlaneStatus struct {
143143
// NetworkSpec encapsulates all things related to OCI network.
144144
// +optional
145145
Conditions clusterv1.Conditions `json:"conditions,omitempty"`
146+
147+
// Version represents the current Kubernetes version for the control plane.
148+
// +optional
149+
Version *string `json:"version,omitempty"`
150+
146151
// Initialized denotes whether or not the control plane has the
147152
// uploaded kubernetes config-map.
148153
// +optional

‎exp/api/v1beta1/ocimanagedcontrolplane_webhook.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2021
"k8s.io/apimachinery/pkg/runtime"
22+
"k8s.io/apimachinery/pkg/util/validation/field"
2123
ctrl "sigs.k8s.io/controller-runtime"
2224
"sigs.k8s.io/controller-runtime/pkg/webhook"
2325
)
@@ -49,7 +51,14 @@ func (c *OCIManagedControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error
4951
}
5052

5153
func (c *OCIManagedControlPlane) ValidateCreate() error {
52-
return nil
54+
var allErrs field.ErrorList
55+
if len(c.Name) > 31 {
56+
allErrs = append(allErrs, field.Invalid(field.NewPath("Name"), c.Name, "Name cannot be more than 31 characters"))
57+
}
58+
if len(allErrs) == 0 {
59+
return nil
60+
}
61+
return apierrors.NewInvalid(c.GroupVersionKind().GroupKind(), c.Name, allErrs)
5362
}
5463

5564
func (c *OCIManagedControlPlane) ValidateUpdate(old runtime.Object) error {

‎exp/api/v1beta1/ocimanagedcontrolplane_webhook_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"strings"
2021
"testing"
2122

2223
"github.com/onsi/gomega"
2324
. "github.com/onsi/gomega"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2426
)
2527

2628
func TestOCIManagedControlPlane_CreateDefault(t *testing.T) {
@@ -50,3 +52,45 @@ func TestOCIManagedControlPlane_CreateDefault(t *testing.T) {
5052
})
5153
}
5254
}
55+
56+
func TestOCIManagedControlPlane_ValidateCreate(t *testing.T) {
57+
tests := []struct {
58+
name string
59+
c *OCIManagedControlPlane
60+
errorMgsShouldContain string
61+
expectErr bool
62+
}{
63+
{
64+
name: "shouldn't allow more than 31 characters",
65+
c: &OCIManagedControlPlane{
66+
ObjectMeta: metav1.ObjectMeta{
67+
Name: "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrst",
68+
},
69+
},
70+
errorMgsShouldContain: "Name cannot be more than 31 characters",
71+
expectErr: true,
72+
},
73+
{
74+
name: "should allow less than 31 characters",
75+
c: &OCIManagedControlPlane{
76+
ObjectMeta: metav1.ObjectMeta{
77+
Name: "abcdefghijklmno",
78+
},
79+
},
80+
expectErr: false,
81+
},
82+
}
83+
for _, test := range tests {
84+
t.Run(test.name, func(t *testing.T) {
85+
g := gomega.NewWithT(t)
86+
87+
if test.expectErr {
88+
err := test.c.ValidateCreate()
89+
g.Expect(err).NotTo(gomega.Succeed())
90+
g.Expect(strings.Contains(err.Error(), test.errorMgsShouldContain)).To(gomega.BeTrue())
91+
} else {
92+
g.Expect(test.c.ValidateCreate()).To(gomega.Succeed())
93+
}
94+
})
95+
}
96+
}

‎exp/api/v1beta1/ocimanagedmachinepool_webhook.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2021
"k8s.io/apimachinery/pkg/runtime"
22+
"k8s.io/apimachinery/pkg/util/validation/field"
2123
ctrl "sigs.k8s.io/controller-runtime"
2224
"sigs.k8s.io/controller-runtime/pkg/webhook"
2325
)
@@ -54,7 +56,14 @@ func (m *OCIManagedMachinePool) Default() {
5456
}
5557

5658
func (m *OCIManagedMachinePool) ValidateCreate() error {
57-
return nil
59+
var allErrs field.ErrorList
60+
if len(m.Name) > 31 {
61+
allErrs = append(allErrs, field.Invalid(field.NewPath("Name"), m.Name, "Name cannot be more than 31 characters"))
62+
}
63+
if len(allErrs) == 0 {
64+
return nil
65+
}
66+
return apierrors.NewInvalid(m.GroupVersionKind().GroupKind(), m.Name, allErrs)
5867
}
5968

6069
func (m *OCIManagedMachinePool) ValidateUpdate(old runtime.Object) error {

‎exp/api/v1beta1/ocimanagedmachinepool_webhook_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"strings"
2021
"testing"
2122

2223
"github.com/onsi/gomega"
2324
. "github.com/onsi/gomega"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2426
)
2527

2628
func TestOCIManagedMachinePool_CreateDefault(t *testing.T) {
@@ -52,3 +54,45 @@ func TestOCIManagedMachinePool_CreateDefault(t *testing.T) {
5254
})
5355
}
5456
}
57+
58+
func TestOCIManagedMachinePool_ValidateCreate(t *testing.T) {
59+
tests := []struct {
60+
name string
61+
m *OCIManagedMachinePool
62+
errorMgsShouldContain string
63+
expectErr bool
64+
}{
65+
{
66+
name: "shouldn't allow more than 31 characters",
67+
m: &OCIManagedMachinePool{
68+
ObjectMeta: metav1.ObjectMeta{
69+
Name: "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrst",
70+
},
71+
},
72+
errorMgsShouldContain: "Name cannot be more than 31 characters",
73+
expectErr: true,
74+
},
75+
{
76+
name: "should allow less than 31 characters",
77+
m: &OCIManagedMachinePool{
78+
ObjectMeta: metav1.ObjectMeta{
79+
Name: "abcdefghijklmno",
80+
},
81+
},
82+
expectErr: false,
83+
},
84+
}
85+
for _, test := range tests {
86+
t.Run(test.name, func(t *testing.T) {
87+
g := gomega.NewWithT(t)
88+
89+
if test.expectErr {
90+
err := test.m.ValidateCreate()
91+
g.Expect(err).NotTo(gomega.Succeed())
92+
g.Expect(strings.Contains(err.Error(), test.errorMgsShouldContain)).To(gomega.BeTrue())
93+
} else {
94+
g.Expect(test.m.ValidateCreate()).To(gomega.Succeed())
95+
}
96+
})
97+
}
98+
}

‎exp/api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎exp/controllers/ocimanaged_machinepool_controller.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,14 @@ func (r *OCIManagedMachinePoolReconciler) reconcileDelete(ctx context.Context, m
347347
}
348348
}
349349

350+
if nodePool == nil {
351+
machinePoolScope.Info("Node Pool is not found, may have been deleted")
352+
controllerutil.RemoveFinalizer(machinePoolScope.OCIManagedMachinePool, infrav1exp.ManagedMachinePoolFinalizer)
353+
conditions.MarkFalse(machinePool, infrav1exp.NodePoolReadyCondition, infrav1exp.NodePoolDeletedReason, clusterv1.ConditionSeverityWarning, "")
354+
return reconcile.Result{}, nil
355+
}
356+
357+
machinePoolScope.Info(fmt.Sprintf("Node Pool lifecycle state is %v", nodePool.LifecycleState))
350358
switch nodePool.LifecycleState {
351359
case oke.NodePoolLifecycleStateDeleting:
352360
// Node Pool is already deleting

0 commit comments

Comments
 (0)