Skip to content

Commit

Permalink
Fix OKE templates and add e2e tests
Browse files Browse the repository at this point in the history
  • Loading branch information
shyamradhakrishnan committed Jan 5, 2023
1 parent e31d67a commit 73e2a44
Show file tree
Hide file tree
Showing 21 changed files with 470 additions and 11 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ generate-e2e-templates: $(KUSTOMIZE)
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-remote-vcn-peering --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-remote-vcn-peering.yaml
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-externally-managed-vcn --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-externally-managed-vcn.yaml
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-machine-pool --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-machine-pool.yaml
$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-managed --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-managed.yaml

.PHONY: test-e2e-run
test-e2e-run: generate-e2e-templates $(GINKGO) $(ENVSUBST) ## Run e2e tests
Expand Down
12 changes: 8 additions & 4 deletions cloud/scope/managed_machine_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (m *ManagedMachinePoolScope) FindNodePool(ctx context.Context) (*oke.NodePo
reqList := oke.ListNodePoolsRequest{
CompartmentId: common.String(m.OCIManagedCluster.Spec.CompartmentId),
ClusterId: m.OCIManagedControlPlane.Spec.ID,
Name: common.String(m.OCIManagedMachinePool.GetName()),
Name: common.String(m.getNodePoolName()),
Page: page,
}

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

func (m *ManagedMachinePoolScope) getNodePoolName() string {
return m.OCIManagedMachinePool.GetName()
}

// CreateNodePool attempts to create a node pool
func (m *ManagedMachinePoolScope) CreateNodePool(ctx context.Context) (*oke.NodePool, error) {
m.Info("Creating Node Pool")
Expand Down Expand Up @@ -285,7 +289,7 @@ func (m *ManagedMachinePoolScope) CreateNodePool(ctx context.Context) (*oke.Node
nodePoolDetails := oke.CreateNodePoolDetails{
CompartmentId: common.String(m.OCIManagedCluster.Spec.CompartmentId),
ClusterId: m.OCIManagedControlPlane.Spec.ID,
Name: common.String(m.OCIManagedMachinePool.Name),
Name: common.String(m.getNodePoolName()),
KubernetesVersion: m.OCIManagedMachinePool.Spec.Version,
NodeShape: common.String(m.OCIManagedMachinePool.Spec.NodeShape),
NodeShapeConfig: &nodeShapeConfig,
Expand Down Expand Up @@ -506,7 +510,7 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
}
actual := m.getSpecFromAPIObject(pool)
if !reflect.DeepEqual(spec, actual) ||
m.OCIManagedMachinePool.Name != *pool.Name || nodePoolSizeUpdateRequired {
m.getNodePoolName() != *pool.Name || nodePoolSizeUpdateRequired {
m.Logger.Info("Updating node pool")
// printing json specs will help debug problems when there are spurious/unwanted updates
jsonSpec, err := json.Marshal(*spec)
Expand Down Expand Up @@ -574,7 +578,7 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
}
}
nodePoolDetails := oke.UpdateNodePoolDetails{
Name: common.String(m.OCIManagedMachinePool.Name),
Name: common.String(m.getNodePoolName()),
KubernetesVersion: m.OCIManagedMachinePool.Spec.Version,
NodeShape: common.String(m.OCIManagedMachinePool.Spec.NodeShape),
NodeShapeConfig: &nodeShapeConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ spec:
type: boolean
ready:
type: boolean
version:
description: Version represents the current Kubernetes version for
the control plane.
type: string
type: object
type: object
served: true
Expand Down
5 changes: 5 additions & 0 deletions exp/api/v1beta1/ocimanagedcontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ type OCIManagedControlPlaneStatus struct {
// NetworkSpec encapsulates all things related to OCI network.
// +optional
Conditions clusterv1.Conditions `json:"conditions,omitempty"`

// Version represents the current Kubernetes version for the control plane.
// +optional
Version *string `json:"version,omitempty"`

// Initialized denotes whether or not the control plane has the
// uploaded kubernetes config-map.
// +optional
Expand Down
11 changes: 10 additions & 1 deletion exp/api/v1beta1/ocimanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package v1beta1

import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)
Expand Down Expand Up @@ -49,7 +51,14 @@ func (c *OCIManagedControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error
}

func (c *OCIManagedControlPlane) ValidateCreate() error {
return nil
var allErrs field.ErrorList
if len(c.Name) > 31 {
allErrs = append(allErrs, field.Invalid(field.NewPath("Name"), c.Name, "Name cannot be more than 31 characters"))
}
if len(allErrs) == 0 {
return nil
}
return apierrors.NewInvalid(c.GroupVersionKind().GroupKind(), c.Name, allErrs)
}

func (c *OCIManagedControlPlane) ValidateUpdate(old runtime.Object) error {
Expand Down
44 changes: 44 additions & 0 deletions exp/api/v1beta1/ocimanagedcontrolplane_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ limitations under the License.
package v1beta1

import (
"strings"
"testing"

"github.com/onsi/gomega"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestOCIManagedControlPlane_CreateDefault(t *testing.T) {
Expand Down Expand Up @@ -50,3 +52,45 @@ func TestOCIManagedControlPlane_CreateDefault(t *testing.T) {
})
}
}

func TestOCIManagedControlPlane_ValidateCreate(t *testing.T) {
tests := []struct {
name string
c *OCIManagedControlPlane
errorMgsShouldContain string
expectErr bool
}{
{
name: "shouldn't allow more than 31 characters",
c: &OCIManagedControlPlane{
ObjectMeta: metav1.ObjectMeta{
Name: "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrst",
},
},
errorMgsShouldContain: "Name cannot be more than 31 characters",
expectErr: true,
},
{
name: "should allow less than 31 characters",
c: &OCIManagedControlPlane{
ObjectMeta: metav1.ObjectMeta{
Name: "abcdefghijklmno",
},
},
expectErr: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := gomega.NewWithT(t)

if test.expectErr {
err := test.c.ValidateCreate()
g.Expect(err).NotTo(gomega.Succeed())
g.Expect(strings.Contains(err.Error(), test.errorMgsShouldContain)).To(gomega.BeTrue())
} else {
g.Expect(test.c.ValidateCreate()).To(gomega.Succeed())
}
})
}
}
11 changes: 10 additions & 1 deletion exp/api/v1beta1/ocimanagedmachinepool_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package v1beta1

import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)
Expand Down Expand Up @@ -54,7 +56,14 @@ func (m *OCIManagedMachinePool) Default() {
}

func (m *OCIManagedMachinePool) ValidateCreate() error {
return nil
var allErrs field.ErrorList
if len(m.Name) > 31 {
allErrs = append(allErrs, field.Invalid(field.NewPath("Name"), m.Name, "Name cannot be more than 31 characters"))
}
if len(allErrs) == 0 {
return nil
}
return apierrors.NewInvalid(m.GroupVersionKind().GroupKind(), m.Name, allErrs)
}

func (m *OCIManagedMachinePool) ValidateUpdate(old runtime.Object) error {
Expand Down
44 changes: 44 additions & 0 deletions exp/api/v1beta1/ocimanagedmachinepool_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ limitations under the License.
package v1beta1

import (
"strings"
"testing"

"github.com/onsi/gomega"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestOCIManagedMachinePool_CreateDefault(t *testing.T) {
Expand Down Expand Up @@ -52,3 +54,45 @@ func TestOCIManagedMachinePool_CreateDefault(t *testing.T) {
})
}
}

func TestOCIManagedMachinePool_ValidateCreate(t *testing.T) {
tests := []struct {
name string
m *OCIManagedMachinePool
errorMgsShouldContain string
expectErr bool
}{
{
name: "shouldn't allow more than 31 characters",
m: &OCIManagedMachinePool{
ObjectMeta: metav1.ObjectMeta{
Name: "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrst",
},
},
errorMgsShouldContain: "Name cannot be more than 31 characters",
expectErr: true,
},
{
name: "should allow less than 31 characters",
m: &OCIManagedMachinePool{
ObjectMeta: metav1.ObjectMeta{
Name: "abcdefghijklmno",
},
},
expectErr: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := gomega.NewWithT(t)

if test.expectErr {
err := test.m.ValidateCreate()
g.Expect(err).NotTo(gomega.Succeed())
g.Expect(strings.Contains(err.Error(), test.errorMgsShouldContain)).To(gomega.BeTrue())
} else {
g.Expect(test.m.ValidateCreate()).To(gomega.Succeed())
}
})
}
}
5 changes: 5 additions & 0 deletions exp/api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions exp/controllers/ocimanaged_machinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,14 @@ func (r *OCIManagedMachinePoolReconciler) reconcileDelete(ctx context.Context, m
}
}

if nodePool == nil {
machinePoolScope.Info("Node Pool is not found, may have been deleted")
controllerutil.RemoveFinalizer(machinePoolScope.OCIManagedMachinePool, infrav1exp.ManagedMachinePoolFinalizer)
conditions.MarkFalse(machinePool, infrav1exp.NodePoolReadyCondition, infrav1exp.NodePoolDeletedReason, clusterv1.ConditionSeverityWarning, "")
return reconcile.Result{}, nil
}

machinePoolScope.Info(fmt.Sprintf("Node Pool lifecycle state is %v", nodePool.LifecycleState))
switch nodePool.LifecycleState {
case oke.NodePoolLifecycleStateDeleting:
// Node Pool is already deleting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ func (r *OCIManagedClusterControlPlaneReconciler) reconcile(ctx context.Context,
Port: 6443,
}
}
controlPlane.Status.Version = okeControlPlane.KubernetesVersion
// record the event only when machine goes from not ready to ready state
r.Recorder.Eventf(controlPlane, corev1.EventTypeNormal, "ControlPlaneReady",
"Managed control plane is in ready state")
Expand Down
4 changes: 2 additions & 2 deletions scripts/ci-e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ source "${REPO_ROOT}/hack/ensure-tags.sh"
: "${OCI_ORACLE_LINUX_IMAGE_ID:?Environment variable empty or not defined.}"
: "${OCI_UPGRADE_IMAGE_ID:?Environment variable empty or not defined.}"
: "${OCI_ALTERNATIVE_REGION_IMAGE_ID:?Environment variable empty or not defined.}"

: OCI_MANAGED_NODE_IMAGE_ID
export LOCAL_ONLY=${LOCAL_ONLY:-"true"}

defaultTag=$(date -u '+%Y%m%d%H%M%S')
Expand All @@ -36,7 +36,7 @@ export OCI_CONTROL_PLANE_MACHINE_TYPE_OCPUS="${OCI_CONTROL_PLANE_MACHINE_TYPE_OC
export OCI_NODE_MACHINE_TYPE="${OCI_NODE_MACHINE_TYPE:-"VM.Standard.E3.Flex"}"
export OCI_NODE_MACHINE_TYPE_OCPUS="${OCI_NODE_MACHINE_TYPE_OCPUS:-"1"}"
export KIND_EXPERIMENTAL_DOCKER_NETWORK="bridge"

export OCI_MANAGED_NODE_SHAPE="${OCI_NODE_MACHINE_TYPE:-"VM.Standard.E4.Flex"}"
export OCI_ALTERNATIVE_REGION="${OCI_ALTERNATIVE_REGION:-"us-sanjose-1"}"

# Generate SSH key.
Expand Down
6 changes: 6 additions & 0 deletions scripts/oke-custom-cloud-init.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/bash
# This script is a custom cloud init which has to be passed during oke nodepool creation while using CAPI
# CAPI needs Kubernetes node provider ID to be of format oci://<instance-id>, by default OKE sets it as <instance-id>
curl --fail -H "Authorization: Bearer Oracle" -L0 http://169.254.169.254/opc/v2/instance/metadata/oke_init_script | base64 --decode >/var/run/oke-init.sh
provider_id=$(curl --fail -H "Authorization: Bearer Oracle" -L0 http://169.254.169.254/opc/v2/instance/id)
bash /var/run/oke-init.sh --kubelet-extra-args "--provider-id=oci://$provider_id"
5 changes: 4 additions & 1 deletion templates/cluster-template-managed-private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,11 @@ metadata:
namespace: default
spec:
version: "${KUBERNETES_VERSION}"
nodeShape: "${OCI_MANAGED_NODE_SHAPE}"
nodeShape: "${OCI_MANAGED_NODE_SHAPE=VM.Standard.E4.Flex}"
sshPublicKey: "${OCI_SSH_KEY}"
nodeMetadata:
# The custom cloud int script generated from the script scripts.oke-custom-cloud-init.sh
user_data: "IyEvYmluL2Jhc2gKY3VybCAtLWZhaWwgLUggIkF1dGhvcml6YXRpb246IEJlYXJlciBPcmFjbGUiIC1MMCBodHRwOi8vMTY5LjI1NC4xNjkuMjU0L29wYy92Mi9pbnN0YW5jZS9tZXRhZGF0YS9va2VfaW5pdF9zY3JpcHQgfCBiYXNlNjQgLS1kZWNvZGUgPi92YXIvcnVuL29rZS1pbml0LnNoCnByb3ZpZGVyX2lkPSQoY3VybCAtLWZhaWwgLUggIkF1dGhvcml6YXRpb246IEJlYXJlciBPcmFjbGUiIC1MMCBodHRwOi8vMTY5LjI1NC4xNjkuMjU0L29wYy92Mi9pbnN0YW5jZS9pZCkKYmFzaCAvdmFyL3J1bi9va2UtaW5pdC5zaCAtLWt1YmVsZXQtZXh0cmEtYXJncyAiLS1wcm92aWRlci1pZD1vY2k6Ly8kcHJvdmlkZXJfaWQiCg=="
nodeSourceViaImage:
imageId: "${OCI_MANAGED_NODE_IMAGE_ID}"
bootVolumeSizeInGBs: ${OCI_MANAGED_NODE_BOOT_VOLUME_SIZE=50}
Expand Down
5 changes: 4 additions & 1 deletion templates/cluster-template-managed.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,11 @@ metadata:
namespace: default
spec:
version: "${KUBERNETES_VERSION}"
nodeShape: "${OCI_MANAGED_NODE_SHAPE}"
nodeShape: "${OCI_MANAGED_NODE_SHAPE=VM.Standard.E4.Flex}"
sshPublicKey: "${OCI_SSH_KEY}"
nodeMetadata:
# The custom cloud int script generated from the script scripts.oke-custom-cloud-init.sh
user_data: "IyEvYmluL2Jhc2gKY3VybCAtLWZhaWwgLUggIkF1dGhvcml6YXRpb246IEJlYXJlciBPcmFjbGUiIC1MMCBodHRwOi8vMTY5LjI1NC4xNjkuMjU0L29wYy92Mi9pbnN0YW5jZS9tZXRhZGF0YS9va2VfaW5pdF9zY3JpcHQgfCBiYXNlNjQgLS1kZWNvZGUgPi92YXIvcnVuL29rZS1pbml0LnNoCnByb3ZpZGVyX2lkPSQoY3VybCAtLWZhaWwgLUggIkF1dGhvcml6YXRpb246IEJlYXJlciBPcmFjbGUiIC1MMCBodHRwOi8vMTY5LjI1NC4xNjkuMjU0L29wYy92Mi9pbnN0YW5jZS9pZCkKYmFzaCAvdmFyL3J1bi9va2UtaW5pdC5zaCAtLWt1YmVsZXQtZXh0cmEtYXJncyAiLS1wcm92aWRlci1pZD1vY2k6Ly8kcHJvdmlkZXJfaWQiCg=="
nodeSourceViaImage:
imageId: "${OCI_MANAGED_NODE_IMAGE_ID}"
bootVolumeSizeInGBs: ${OCI_MANAGED_NODE_BOOT_VOLUME_SIZE=50}
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/config/e2e_conf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,13 @@ providers:
- sourcePath: "../data/infrastructure-oci/v1beta1/cluster-template-remote-vcn-peering.yaml"
- sourcePath: "../data/infrastructure-oci/v1beta1/cluster-template-externally-managed-vcn.yaml"
- sourcePath: "../data/infrastructure-oci/v1beta1/cluster-template-machine-pool.yaml"
- sourcePath: "../data/infrastructure-oci/v1beta1/cluster-template-managed.yaml"
- sourcePath: "../data/infrastructure-oci/v1beta1/metadata.yaml"

variables:
KUBERNETES_VERSION: "v1.24.4"
OCI_MANAGED_KUBERNETES_VERSION: "v1.23.4"
OCI_MANAGED_KUBERNETES_VERSION_UPGRADE: "v1.24.1"
EXP_MACHINE_POOL: "true"
EXP_CLUSTER_RESOURCE_SET: "true"
NODE_DRAIN_TIMEOUT: "60s"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
apiVersion: cluster.x-k8s.io/v1beta1
kind: Cluster
metadata:
labels:
cluster.x-k8s.io/cluster-name: "${CLUSTER_NAME}"
name: "${CLUSTER_NAME}"
namespace: "${NAMESPACE}"
spec:
infrastructureRef:
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: OCIManagedCluster
name: "${CLUSTER_NAME}"
namespace: "${NAMESPACE}"
controlPlaneRef:
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: OCIManagedControlPlane
name: "${CLUSTER_NAME}"
namespace: "${NAMESPACE}"
---
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: OCIManagedCluster
metadata:
labels:
cluster.x-k8s.io/cluster-name: "${CLUSTER_NAME}"
name: "${CLUSTER_NAME}"
spec:
compartmentId: "${OCI_COMPARTMENT_ID}"
---
kind: OCIManagedControlPlane
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
metadata:
name: "${CLUSTER_NAME}"
namespace: "${NAMESPACE}"
spec:
version: "${OCI_MANAGED_KUBERNETES_VERSION}"
---
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
bases:
- ./cluster.yaml
- ./machine-pool.yaml


Loading

0 comments on commit 73e2a44

Please sign in to comment.