From 58e1a28810869aa09ace2d2e1e21796a97b4430a Mon Sep 17 00:00:00 2001 From: Joe Kratzat Date: Fri, 6 Jan 2023 15:37:02 -0500 Subject: [PATCH] feat: add VnicAttachments reconciler This allows users to define vnic attachments. The reconciler will then attach them after the instance is launched. --- Makefile | 1 + Tiltfile | 2 +- api/v1beta1/conditions_consts.go | 4 + api/v1beta1/ocimachine_types.go | 4 + api/v1beta1/types.go | 24 ++ api/v1beta1/zz_generated.deepcopy.go | 37 +++ cloud/scope/machine.go | 30 +-- cloud/scope/util.go | 17 ++ cloud/scope/vnic_reconciler.go | 129 +++++++++++ cloud/scope/vnic_reconciler_test.go | 213 ++++++++++++++++++ cloud/services/compute/client.go | 1 + .../compute/mock_compute/client_mock.go | 15 ++ cloud/services/vcn/client.go | 1 + cloud/services/vcn/mock_vcn/client_mock.go | 15 ++ ...tructure.cluster.x-k8s.io_ocimachines.yaml | 30 +++ ....cluster.x-k8s.io_ocimachinetemplates.yaml | 34 +++ controllers/ocimachine_controller.go | 14 ++ scripts/ci-e2e.sh | 2 + .../cluster-template-windows-calico.yaml | 156 +++++++++++++ test/e2e/cluster_test.go | 50 ++++ test/e2e/config/e2e_conf.yaml | 2 + .../kustomization.yaml | 7 + .../cluster-template-windows-calico/md.yaml | 46 ++++ 23 files changed, 820 insertions(+), 14 deletions(-) create mode 100644 cloud/scope/vnic_reconciler.go create mode 100644 cloud/scope/vnic_reconciler_test.go create mode 100644 templates/cluster-template-windows-calico.yaml create mode 100644 test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/kustomization.yaml create mode 100644 test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/md.yaml diff --git a/Makefile b/Makefile index bb1b9e0b..49feae00 100644 --- a/Makefile +++ b/Makefile @@ -253,6 +253,7 @@ generate-e2e-templates: $(KUSTOMIZE) $(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 + $(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta1/cluster-template-windows-calico --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta1/cluster-template-windows-calico.yaml .PHONY: test-e2e-run test-e2e-run: generate-e2e-templates $(GINKGO) $(ENVSUBST) ## Run e2e tests diff --git a/Tiltfile b/Tiltfile index 16635301..a69726af 100644 --- a/Tiltfile +++ b/Tiltfile @@ -41,7 +41,7 @@ if "default_registry" in settings: tilt_helper_dockerfile_header = """ # Tilt image -FROM golang:1.17 as tilt-helper +FROM golang:1.18 as tilt-helper # Support live reloading with Tilt RUN wget --output-document /restart.sh --quiet https://raw.githubusercontent.com/windmilleng/rerun-process-wrapper/master/restart.sh && \ wget --output-document /start.sh --quiet https://raw.githubusercontent.com/windmilleng/rerun-process-wrapper/master/start.sh && \ diff --git a/api/v1beta1/conditions_consts.go b/api/v1beta1/conditions_consts.go index b33dc46d..daf53b71 100644 --- a/api/v1beta1/conditions_consts.go +++ b/api/v1beta1/conditions_consts.go @@ -69,6 +69,8 @@ const ( FailureDomainFailedReason = "FailureDomainFailedReconciliationFailed" // InstanceLBBackendAdditionFailedReason used when addition to LB backend fails InstanceLBBackendAdditionFailedReason = "BackendAdditionFailed" + // InstanceVnicAttachmentFailedReason used when attaching vnics to machine + InstanceVnicAttachmentFailedReason = "VnicAttachmentFailed" // InstanceIPAddressNotFound used when IP address of the instance count not be found InstanceIPAddressNotFound = "InstanceIPAddressNotFound" // VcnEventReady used after reconciliation has completed successfully @@ -91,6 +93,8 @@ const ( RouteTableEventReady = "RouteTableReady" // SubnetEventReady used after reconciliation has completed successfully SubnetEventReady = "SubnetReady" + // InstanceVnicAttachmentReady used after reconciliation has been completed successfully + InstanceVnicAttachmentReady = "VnicAttachmentReady" // ApiServerLoadBalancerEventReady used after reconciliation has completed successfully ApiServerLoadBalancerEventReady = "APIServerLoadBalancerReady" // FailureDomainEventReady used after reconciliation has completed successfully diff --git a/api/v1beta1/ocimachine_types.go b/api/v1beta1/ocimachine_types.go index 8531d32b..bc97fb0a 100644 --- a/api/v1beta1/ocimachine_types.go +++ b/api/v1beta1/ocimachine_types.go @@ -61,6 +61,10 @@ type OCIMachineSpec struct { // NetworkDetails defines the configuration options for the network NetworkDetails NetworkDetails `json:"networkDetails,omitempty"` + // VnicAttachments defines the configuration options for the vnic(s) attached to the machine + // The network bandwidth and number of VNICs scale proportionately with the number of OCPUs. + VnicAttachments []VnicAttachment `json:"vnicAttachments,omitempty"` + // LaunchOptions defines the options for tuning the compatibility and performance of VM shapes LaunchOptions *LaunchOptions `json:"launchOptions,omitempty"` diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 09719e95..663e9a50 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -64,6 +64,30 @@ type NetworkDetails struct { AssignPrivateDnsRecord *bool `json:"assignPrivateDnsRecord,omitempty"` } +type VnicAttachment struct { + // VnicAttachmentId defines the ID of the VnicAttachment + VnicAttachmentId *string `json:"vnicAttachmentId,omitempty"` + + // AssignPublicIp defines whether the vnic should have a public IP address + // +optional + AssignPublicIp bool `json:"assignPublicIp,omitempty"` + + // SubnetName defines the subnet name to use for the VNIC + // Defaults to the "worker" subnet if not provided + // +optional + SubnetName string `json:"subnetName,omitempty"` + + // DisplayName defines a user-friendly name. Does not have to be unique. + // Avoid entering confidential information. + DisplayName *string `json:"displayName"` + + // NicIndex defines which physical Network Interface Card (NIC) to use + // You can determine which NICs are active for a shape by reviewing the + // https://docs.oracle.com/en-us/iaas/Content/Compute/References/computeshapes.htm + // +optional + NicIndex *int `json:"nicIndex,omitempty"` +} + // LaunchOptionsBootVolumeTypeEnum Enum with underlying type: string type LaunchOptionsBootVolumeTypeEnum string diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index e45fc53f..1d9ef125 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1107,6 +1107,13 @@ func (in *OCIMachineSpec) DeepCopyInto(out *OCIMachineSpec) { } in.ShapeConfig.DeepCopyInto(&out.ShapeConfig) in.NetworkDetails.DeepCopyInto(&out.NetworkDetails) + if in.VnicAttachments != nil { + in, out := &in.VnicAttachments, &out.VnicAttachments + *out = make([]VnicAttachment, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.LaunchOptions != nil { in, out := &in.LaunchOptions, &out.LaunchOptions *out = new(LaunchOptions) @@ -1681,3 +1688,33 @@ func (in *VCNPeering) DeepCopy() *VCNPeering { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VnicAttachment) DeepCopyInto(out *VnicAttachment) { + *out = *in + if in.VnicAttachmentId != nil { + in, out := &in.VnicAttachmentId, &out.VnicAttachmentId + *out = new(string) + **out = **in + } + if in.DisplayName != nil { + in, out := &in.DisplayName, &out.DisplayName + *out = new(string) + **out = **in + } + if in.NicIndex != nil { + in, out := &in.NicIndex, &out.NicIndex + *out = new(int) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VnicAttachment. +func (in *VnicAttachment) DeepCopy() *VnicAttachment { + if in == nil { + return nil + } + out := new(VnicAttachment) + in.DeepCopyInto(out) + return out +} diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index f2ecc935..c5567cf6 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -220,16 +220,7 @@ func (m *MachineScope) GetOrCreateMachine(ctx context.Context) (*core.Instance, tags := m.getFreeFormTags(*m.OCICluster) - definedTags := make(map[string]map[string]interface{}) - if m.OCIMachine.Spec.DefinedTags != nil { - for ns, mapNs := range m.OCIMachine.Spec.DefinedTags { - mapValues := make(map[string]interface{}) - for k, v := range mapNs { - mapValues[k] = v - } - definedTags[ns] = mapValues - } - } + definedTags := ConvertMachineDefinedTags(m.OCIMachine.Spec.DefinedTags) availabilityDomain := m.OCICluster.Status.FailureDomains[*failureDomain].Attributes[AvailabilityDomain] faultDomain := m.OCICluster.Status.FailureDomains[*failureDomain].Attributes[FaultDomain] @@ -309,8 +300,8 @@ func (m *MachineScope) DeleteMachine(ctx context.Context) error { // IsResourceCreatedByClusterAPI determines if the instance was created by the cluster using the // tags created at instance launch. -func (s *MachineScope) IsResourceCreatedByClusterAPI(resourceFreeFormTags map[string]string) bool { - tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(s.OCICluster.GetOCIResourceIdentifier())) +func (m *MachineScope) IsResourceCreatedByClusterAPI(resourceFreeFormTags map[string]string) bool { + tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(m.OCICluster.GetOCIResourceIdentifier())) for k, v := range tagsAddedByClusterAPI { if resourceFreeFormTags[k] != v { return false @@ -474,8 +465,10 @@ func (m *MachineScope) GetInstanceIp(ctx context.Context) (*string, error) { } } - if page = resp.OpcNextPage; resp.OpcNextPage == nil { + if resp.OpcNextPage == nil { break + } else { + page = resp.OpcNextPage } } @@ -620,6 +613,17 @@ func (m *MachineScope) getGetControlPlaneMachineNSGs() []string { return nsgs } +// getMachineSubnet iterates through the OCICluster Vcn subnets +// and returns the subnet ID if the name matches +func (m *MachineScope) getMachineSubnet(name string) (*string, error) { + for _, subnet := range m.OCICluster.Spec.NetworkSpec.Vcn.Subnets { + if subnet.Name == name { + return subnet.ID, nil + } + } + return nil, errors.New(fmt.Sprintf("Subnet with name %s not found for cluster %s", name, m.OCICluster.Name)) +} + func (m *MachineScope) getWorkerMachineSubnet() *string { for _, subnet := range m.OCICluster.Spec.NetworkSpec.Vcn.Subnets { if subnet.Role == infrastructurev1beta1.WorkerRole { diff --git a/cloud/scope/util.go b/cloud/scope/util.go index 0e49d5c7..92f7ee5f 100644 --- a/cloud/scope/util.go +++ b/cloud/scope/util.go @@ -55,3 +55,20 @@ func GetSubnetNamesFromId(ids []string, subnets []*infrastructurev1beta1.Subnet) } return names } + +// ConvertMachineDefinedTags passes in the OCIMachineSpec DefinedTags and returns a converted map of defined tags +// to be used when creating API requests. +func ConvertMachineDefinedTags(machineDefinedTags map[string]map[string]string) map[string]map[string]interface{} { + definedTags := make(map[string]map[string]interface{}) + if machineDefinedTags != nil { + for ns, mapNs := range machineDefinedTags { + mapValues := make(map[string]interface{}) + for k, v := range mapNs { + mapValues[k] = v + } + definedTags[ns] = mapValues + } + } + + return definedTags +} diff --git a/cloud/scope/vnic_reconciler.go b/cloud/scope/vnic_reconciler.go new file mode 100644 index 00000000..60ad9e12 --- /dev/null +++ b/cloud/scope/vnic_reconciler.go @@ -0,0 +1,129 @@ +/* + Copyright (c) 2022 Oracle and/or its affiliates. + + 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 + + https://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. +*/ + +package scope + +import ( + "context" + "fmt" + "github.com/oracle/cluster-api-provider-oci/cloud/ociutil" + + infrastructurev1beta1 "github.com/oracle/cluster-api-provider-oci/api/v1beta1" + "github.com/oracle/oci-go-sdk/v65/common" + "github.com/oracle/oci-go-sdk/v65/core" + "github.com/pkg/errors" +) + +func (m *MachineScope) ReconcileVnicAttachments(ctx context.Context) error { + if m.IsControlPlane() { + return errors.New("cannot attach multiple vnics to ControlPlane machines") + } + + for index, vnicAttachment := range m.OCIMachine.Spec.VnicAttachments { + if m.vnicAttachmentExists(ctx, vnicAttachment) { + m.Logger.Info("vnicAttachment", ociutil.DerefString(vnicAttachment.DisplayName), " already exists and is immutable") + continue + } + + vnicId, err := m.createVnicAttachment(ctx, vnicAttachment) + if err != nil { + msg := fmt.Sprintf("Error creating VnicAttachment %s for cluster %s", + *vnicAttachment.DisplayName, m.Cluster.Name) + m.Logger.Error(err, msg) + return err + } + + m.OCIMachine.Spec.VnicAttachments[index].VnicAttachmentId = vnicId + } + + return nil +} + +func (m *MachineScope) createVnicAttachment(ctx context.Context, spec infrastructurev1beta1.VnicAttachment) (*string, error) { + vnicName := spec.DisplayName + + // Default to machine subnet if spec doesn't supply one + subnetId := m.getWorkerMachineSubnet() + if spec.SubnetName != "" { + var err error + subnetId, err = m.getMachineSubnet(spec.SubnetName) + if err != nil { + return nil, err + } + } + + tags := m.getFreeFormTags(*m.OCICluster) + + definedTags := ConvertMachineDefinedTags(m.OCIMachine.Spec.DefinedTags) + + if spec.NicIndex == nil { + spec.NicIndex = common.Int(0) + } + + secondVnic := core.AttachVnicDetails{ + DisplayName: vnicName, + NicIndex: spec.NicIndex, + InstanceId: m.OCIMachine.Spec.InstanceId, + CreateVnicDetails: &core.CreateVnicDetails{ + SubnetId: subnetId, + AssignPublicIp: common.Bool(spec.AssignPublicIp), + FreeformTags: tags, + DefinedTags: definedTags, + HostnameLabel: m.OCIMachine.Spec.NetworkDetails.HostnameLabel, + NsgIds: m.getWorkerMachineNSGs(), + SkipSourceDestCheck: m.OCIMachine.Spec.NetworkDetails.SkipSourceDestCheck, + AssignPrivateDnsRecord: m.OCIMachine.Spec.NetworkDetails.AssignPrivateDnsRecord, + DisplayName: vnicName, + }, + } + + req := core.AttachVnicRequest{AttachVnicDetails: secondVnic} + resp, err := m.ComputeClient.AttachVnic(ctx, req) + if err != nil { + return nil, err + } + + return resp.Id, nil +} + +func (m *MachineScope) vnicAttachmentExists(ctx context.Context, vnic infrastructurev1beta1.VnicAttachment) bool { + + found := false + var page *string + for { + resp, err := m.ComputeClient.ListVnicAttachments(ctx, core.ListVnicAttachmentsRequest{ + InstanceId: m.GetInstanceId(), + CompartmentId: common.String(m.getCompartmentId()), + Page: page, + }) + if err != nil { + return false + } + for _, attachment := range resp.Items { + if ociutil.DerefString(attachment.DisplayName) == ociutil.DerefString(vnic.DisplayName) { + m.Logger.Info("Vnic is already attached ", attachment) + return true + } + } + + if resp.OpcNextPage == nil { + break + } else { + page = resp.OpcNextPage + } + } + return found +} diff --git a/cloud/scope/vnic_reconciler_test.go b/cloud/scope/vnic_reconciler_test.go new file mode 100644 index 00000000..d18caba7 --- /dev/null +++ b/cloud/scope/vnic_reconciler_test.go @@ -0,0 +1,213 @@ +/* + Copyright (c) 2022 Oracle and/or its affiliates. + + 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 + + https://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. +*/ + +package scope + +import ( + "context" + "fmt" + "testing" + + "github.com/golang/mock/gomock" + . "github.com/onsi/gomega" + infrastructurev1beta1 "github.com/oracle/cluster-api-provider-oci/api/v1beta1" + "github.com/oracle/cluster-api-provider-oci/cloud/ociutil" + "github.com/oracle/cluster-api-provider-oci/cloud/services/compute/mock_compute" + "github.com/oracle/oci-go-sdk/v65/common" + "github.com/oracle/oci-go-sdk/v65/core" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestReconcileVnicAttachment(t *testing.T) { + var ( + ms *MachineScope + mockCtrl *gomock.Controller + computeClient *mock_compute.MockComputeClient + ociCluster infrastructurev1beta1.OCICluster + ) + + setup := func(t *testing.T, g *WithT) { + var err error + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bootstrap", + Namespace: "default", + }, + Data: map[string][]byte{ + "value": []byte("test"), + }, + } + + mockCtrl = gomock.NewController(t) + computeClient = mock_compute.NewMockComputeClient(mockCtrl) + client := fake.NewClientBuilder().WithObjects(secret).Build() + ociCluster = infrastructurev1beta1.OCICluster{ + ObjectMeta: metav1.ObjectMeta{ + UID: "uid", + }, + Spec: infrastructurev1beta1.OCIClusterSpec{ + OCIResourceIdentifier: "resource_uid", + }, + } + ociCluster.Spec.ControlPlaneEndpoint.Port = 6443 + ms, err = NewMachineScope(MachineScopeParams{ + ComputeClient: computeClient, + OCIMachine: &infrastructurev1beta1.OCIMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: infrastructurev1beta1.OCIMachineSpec{ + CompartmentId: "testCompartment", + VnicAttachments: []infrastructurev1beta1.VnicAttachment{ + { + DisplayName: common.String("VnicTest"), + NicIndex: common.Int(0), + }, + }, + }, + }, + Machine: &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: common.String("bootstrap"), + }, + }, + }, + Cluster: &clusterv1.Cluster{}, + OCICluster: &ociCluster, + Client: client, + }) + ms.Machine.Namespace = "default" + g.Expect(err).To(BeNil()) + } + teardown := func(t *testing.T, g *WithT) { + mockCtrl.Finish() + + } + + tests := []struct { + name string + errorExpected bool + objects []client.Object + expectedEvent string + eventNotExpected string + matchError error + errorSubStringMatch bool + testSpecificSetup func(machineScope *MachineScope, computeClient *mock_compute.MockComputeClient) + }{ + { + name: "Crete vnic attachment", + errorExpected: false, + testSpecificSetup: func(machineScope *MachineScope, computeClient *mock_compute.MockComputeClient) { + ms.OCIMachine.Spec.InstanceId = common.String("test") + computeClient.EXPECT().ListVnicAttachments(gomock.Any(), gomock.Eq(core.ListVnicAttachmentsRequest{ + InstanceId: common.String("test"), + CompartmentId: common.String("testCompartment"), + })). + Return(core.ListVnicAttachmentsResponse{ + Items: []core.VnicAttachment{ + { + InstanceId: common.String("test"), + DisplayName: common.String("vnicDisplayName"), + }, + { + InstanceId: common.String("test"), + DisplayName: common.String("vnicDisplayName"), + }, + }, + }, nil) + + computeClient.EXPECT().AttachVnic(gomock.Any(), gomock.Eq(core.AttachVnicRequest{ + AttachVnicDetails: core.AttachVnicDetails{ + DisplayName: common.String("VnicTest"), + NicIndex: common.Int(0), + InstanceId: common.String("test"), + CreateVnicDetails: &core.CreateVnicDetails{ + DisplayName: common.String("VnicTest"), + AssignPublicIp: common.Bool(false), + DefinedTags: map[string]map[string]interface{}{}, + FreeformTags: map[string]string{ + ociutil.CreatedBy: ociutil.OCIClusterAPIProvider, + ociutil.ClusterResourceIdentifier: "resource_uid", + }, + NsgIds: nil, + }, + }})). + Return(core.AttachVnicResponse{ + VnicAttachment: core.VnicAttachment{Id: common.String("vnic.id")}, + }, nil) + }, + }, + { + name: "Crete vnic attachment error", + errorExpected: true, + matchError: fmt.Errorf("could not attach to nic 10"), + testSpecificSetup: func(machineScope *MachineScope, computeClient *mock_compute.MockComputeClient) { + ms.OCIMachine.Spec.InstanceId = common.String("test") + computeClient.EXPECT().ListVnicAttachments(gomock.Any(), gomock.Eq(core.ListVnicAttachmentsRequest{ + InstanceId: common.String("test"), + CompartmentId: common.String("testCompartment"), + })). + Return(core.ListVnicAttachmentsResponse{ + Items: []core.VnicAttachment{ + { + InstanceId: common.String("test"), + DisplayName: common.String("vnicDisplayName"), + NicIndex: common.Int(10), + }, + }, + }, nil) + + computeClient.EXPECT().AttachVnic(gomock.Any(), gomock.Any()). + Return(core.AttachVnicResponse{}, errors.New("could not attach to nic 10")) + }, + }, + { + name: "Crete vnic attachment on control plane will fail", + errorExpected: true, + matchError: fmt.Errorf("cannot attach multiple vnics to ControlPlane machines"), + testSpecificSetup: func(machineScope *MachineScope, computeClient *mock_compute.MockComputeClient) { + ms.Machine.ObjectMeta.Labels = make(map[string]string) + ms.Machine.ObjectMeta.Labels[clusterv1.MachineControlPlaneLabelName] = "Test" + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + defer teardown(t, g) + setup(t, g) + tc.testSpecificSetup(ms, computeClient) + err := ms.ReconcileVnicAttachments(context.Background()) + if tc.errorExpected { + g.Expect(err).To(Not(BeNil())) + if tc.errorSubStringMatch { + g.Expect(err.Error()).To(ContainSubstring(tc.matchError.Error())) + } else { + g.Expect(err.Error()).To(Equal(tc.matchError.Error())) + } + } else { + g.Expect(err).To(BeNil()) + } + }) + } +} diff --git a/cloud/services/compute/client.go b/cloud/services/compute/client.go index 0e6ecd0c..58fda158 100644 --- a/cloud/services/compute/client.go +++ b/cloud/services/compute/client.go @@ -27,5 +27,6 @@ type ComputeClient interface { TerminateInstance(ctx context.Context, request core.TerminateInstanceRequest) (response core.TerminateInstanceResponse, err error) GetInstance(ctx context.Context, request core.GetInstanceRequest) (response core.GetInstanceResponse, err error) ListInstances(ctx context.Context, request core.ListInstancesRequest) (response core.ListInstancesResponse, err error) + AttachVnic(ctx context.Context, request core.AttachVnicRequest) (response core.AttachVnicResponse, err error) ListVnicAttachments(ctx context.Context, request core.ListVnicAttachmentsRequest) (response core.ListVnicAttachmentsResponse, err error) } diff --git a/cloud/services/compute/mock_compute/client_mock.go b/cloud/services/compute/mock_compute/client_mock.go index 9a3987d0..56ad89df 100644 --- a/cloud/services/compute/mock_compute/client_mock.go +++ b/cloud/services/compute/mock_compute/client_mock.go @@ -35,6 +35,21 @@ func (m *MockComputeClient) EXPECT() *MockComputeClientMockRecorder { return m.recorder } +// AttachVnic mocks base method. +func (m *MockComputeClient) AttachVnic(ctx context.Context, request core.AttachVnicRequest) (core.AttachVnicResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AttachVnic", ctx, request) + ret0, _ := ret[0].(core.AttachVnicResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AttachVnic indicates an expected call of AttachVnic. +func (mr *MockComputeClientMockRecorder) AttachVnic(ctx, request interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AttachVnic", reflect.TypeOf((*MockComputeClient)(nil).AttachVnic), ctx, request) +} + // GetInstance mocks base method. func (m *MockComputeClient) GetInstance(ctx context.Context, request core.GetInstanceRequest) (core.GetInstanceResponse, error) { m.ctrl.T.Helper() diff --git a/cloud/services/vcn/client.go b/cloud/services/vcn/client.go index 7c37700e..bc1a9095 100644 --- a/cloud/services/vcn/client.go +++ b/cloud/services/vcn/client.go @@ -69,6 +69,7 @@ type Client interface { ListServices(ctx context.Context, request core.ListServicesRequest) (response core.ListServicesResponse, err error) // Vnic GetVnic(ctx context.Context, request core.GetVnicRequest) (response core.GetVnicResponse, err error) + UpdateVnic(ctx context.Context, request core.UpdateVnicRequest) (response core.UpdateVnicResponse, err error) // NSG GetNetworkSecurityGroup(ctx context.Context, request core.GetNetworkSecurityGroupRequest) (response core.GetNetworkSecurityGroupResponse, err error) ListNetworkSecurityGroups(ctx context.Context, request core.ListNetworkSecurityGroupsRequest) (response core.ListNetworkSecurityGroupsResponse, err error) diff --git a/cloud/services/vcn/mock_vcn/client_mock.go b/cloud/services/vcn/mock_vcn/client_mock.go index e80e7ff6..eeb3fc21 100644 --- a/cloud/services/vcn/mock_vcn/client_mock.go +++ b/cloud/services/vcn/mock_vcn/client_mock.go @@ -575,6 +575,21 @@ func (mr *MockClientMockRecorder) GetVnic(ctx, request interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVnic", reflect.TypeOf((*MockClient)(nil).GetVnic), ctx, request) } +// UpdateVnic mocks base method. +func (m *MockClient) UpdateVnic(ctx context.Context, request core.UpdateVnicRequest) (core.UpdateVnicResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateVnic", ctx, request) + ret0, _ := ret[0].(core.UpdateVnicResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateVnic indicates an expected call of UpdateVnic. +func (mr *MockClientMockRecorder) UpdateVnic(ctx, request interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateVnic", reflect.TypeOf((*MockClient)(nil).UpdateVnic), ctx, request) +} + // ListDrgAttachments mocks base method. func (m *MockClient) ListDrgAttachments(ctx context.Context, request core.ListDrgAttachmentsRequest) (core.ListDrgAttachmentsResponse, error) { m.ctrl.T.Helper() diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml index 9312e7f6..e7836910 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml @@ -620,6 +620,36 @@ spec: the subnets defined in the OCICluster Spec. Optional, only if multiple subnets of a type is defined, else the first element is used. type: string + vnicAttachments: + description: VnicAttachments defines the configuration options for + the vnic(s) attached to the machine The network bandwidth and number + of VNICs scale proportionately with the number of OCPUs. + items: + properties: + assignPublicIp: + description: AssignPublicIp defines whether the vnic should + have a public IP address + type: boolean + displayName: + description: DisplayName defines a user-friendly name. Does + not have to be unique. Avoid entering confidential information. + type: string + nicIndex: + description: NicIndex defines which physical Network Interface + Card (NIC) to use You can determine which NICs are active + for a shape by reviewing the https://docs.oracle.com/en-us/iaas/Content/Compute/References/computeshapes.htm + type: integer + subnetName: + description: SubnetName defines the subnet name to use for the + VNIC Defaults to the "worker" subnet if not provided + type: string + vnicAttachmentId: + description: VnicAttachmentId defines the ID of the VnicAttachment + type: string + required: + - displayName + type: object + type: array type: object status: description: OCIMachineStatus defines the observed state of OCIMachine. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml index 79f7db0b..fc48291f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml @@ -672,6 +672,40 @@ spec: only if multiple subnets of a type is defined, else the first element is used. type: string + vnicAttachments: + description: VnicAttachments defines the configuration options + for the vnic(s) attached to the machine The network bandwidth + and number of VNICs scale proportionately with the number + of OCPUs. + items: + properties: + assignPublicIp: + description: AssignPublicIp defines whether the vnic + should have a public IP address + type: boolean + displayName: + description: DisplayName defines a user-friendly name. + Does not have to be unique. Avoid entering confidential + information. + type: string + nicIndex: + description: NicIndex defines which physical Network + Interface Card (NIC) to use You can determine which + NICs are active for a shape by reviewing the https://docs.oracle.com/en-us/iaas/Content/Compute/References/computeshapes.htm + type: integer + subnetName: + description: SubnetName defines the subnet name to use + for the VNIC Defaults to the "worker" subnet if not + provided + type: string + vnicAttachmentId: + description: VnicAttachmentId defines the ID of the + VnicAttachment + type: string + required: + - displayName + type: object + type: array type: object required: - spec diff --git a/controllers/ocimachine_controller.go b/controllers/ocimachine_controller.go index ae6cbdfe..d0bd6a83 100644 --- a/controllers/ocimachine_controller.go +++ b/controllers/ocimachine_controller.go @@ -289,6 +289,20 @@ func (r *OCIMachineReconciler) reconcileNormal(ctx context.Context, logger logr. } machineScope.Info("Instance is added to the control plane LB") } + + if len(machine.Spec.VnicAttachments) > 0 { + err := machineScope.ReconcileVnicAttachments(ctx) + if err != nil { + r.Recorder.Event(machine, corev1.EventTypeWarning, "ReconcileError", errors.Wrapf(err, "failed to reconcile OCIMachine").Error()) + conditions.MarkFalse(machineScope.OCIMachine, infrastructurev1beta1.InstanceReadyCondition, + infrastructurev1beta1.InstanceVnicAttachmentFailedReason, clusterv1.ConditionSeverityError, "") + return ctrl.Result{}, err + } + machineScope.Info("Instance vnic attachment success") + r.Recorder.Eventf(machineScope.OCIMachine, corev1.EventTypeNormal, infrastructurev1beta1.InstanceVnicAttachmentReady, + "VNICs have been attached to instance.") + } + // record the event only when machine goes from not ready to ready state r.Recorder.Eventf(machine, corev1.EventTypeNormal, "InstanceReady", "Instance is in ready state") diff --git a/scripts/ci-e2e.sh b/scripts/ci-e2e.sh index 99e63e5e..14407336 100755 --- a/scripts/ci-e2e.sh +++ b/scripts/ci-e2e.sh @@ -24,6 +24,8 @@ source "${REPO_ROOT}/hack/ensure-tags.sh" : "${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 +: OCI_WINDOWS_IMAGE_ID + export LOCAL_ONLY=${LOCAL_ONLY:-"true"} defaultTag=$(date -u '+%Y%m%d%H%M%S') diff --git a/templates/cluster-template-windows-calico.yaml b/templates/cluster-template-windows-calico.yaml new file mode 100644 index 00000000..a036fd61 --- /dev/null +++ b/templates/cluster-template-windows-calico.yaml @@ -0,0 +1,156 @@ +apiVersion: cluster.x-k8s.io/v1beta1 +kind: Cluster +metadata: + labels: + cluster.x-k8s.io/cluster-name: "${CLUSTER_NAME}" + cni: calico + csi-proxy: enabled + windows: enabled + name: "${CLUSTER_NAME}" + namespace: "${NAMESPACE}" +spec: + clusterNetwork: + pods: + cidrBlocks: + - ${POD_CIDR:="192.168.0.0/16"} + serviceDomain: ${SERVICE_DOMAIN:="cluster.local"} + services: + cidrBlocks: + - ${SERVICE_CIDR:="10.128.0.0/12"} + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 + kind: OCICluster + name: "${CLUSTER_NAME}" + namespace: "${NAMESPACE}" + controlPlaneRef: + apiVersion: controlplane.cluster.x-k8s.io/v1beta1 + kind: KubeadmControlPlane + name: "${CLUSTER_NAME}-control-plane" + namespace: "${NAMESPACE}" +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: OCICluster +metadata: + labels: + cluster.x-k8s.io/cluster-name: "${CLUSTER_NAME}" + name: "${CLUSTER_NAME}" +spec: + compartmentId: "${OCI_COMPARTMENT_ID}" +--- +kind: KubeadmControlPlane +apiVersion: controlplane.cluster.x-k8s.io/v1beta1 +metadata: + name: "${CLUSTER_NAME}-control-plane" + namespace: "${NAMESPACE}" +spec: + version: "${KUBERNETES_VERSION}" + replicas: ${CONTROL_PLANE_MACHINE_COUNT} + machineTemplate: + infrastructureRef: + kind: OCIMachineTemplate + apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 + name: "${CLUSTER_NAME}-control-plane" + namespace: "${NAMESPACE}" + kubeadmConfigSpec: + clusterConfiguration: + kubernetesVersion: ${KUBERNETES_VERSION} + apiServer: + certSANs: [localhost, 127.0.0.1] + extraArgs: + cloud-provider: oci + dns: {} + etcd: {} + networking: {} + scheduler: {} + initConfiguration: + nodeRegistration: + criSocket: /var/run/containerd/containerd.sock + kubeletExtraArgs: + cloud-provider: external + provider-id: oci://{{ ds["id"] }} + joinConfiguration: + discovery: {} + nodeRegistration: + criSocket: /var/run/containerd/containerd.sock + kubeletExtraArgs: + cloud-provider: external + provider-id: oci://{{ ds["id"] }} +--- +kind: OCIMachineTemplate +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +metadata: + name: "${CLUSTER_NAME}-control-plane" +spec: + template: + spec: + imageId: "${OCI_CONTROL_PLANE_IMAGE_ID}" + compartmentId: "${OCI_COMPARTMENT_ID}" + shape: "${OCI_CONTROL_PLANE_MACHINE_TYPE=VM.Standard.E4.Flex}" + shapeConfig: + ocpus: "${OCI_CONTROL_PLANE_MACHINE_TYPE_OCPUS=1}" + metadata: + ssh_authorized_keys: "${OCI_SSH_KEY}" + isPvEncryptionInTransitEnabled: ${OCI_CONTROL_PLANE_PV_TRANSIT_ENCRYPTION=true} +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: OCIMachineTemplate +metadata: + name: "${CLUSTER_NAME}-md-win" +spec: + template: + spec: + imageId: "${OCI_NODE_IMAGE_ID}" + compartmentId: "${OCI_COMPARTMENT_ID}" + shape: "${OCI_NODE_MACHINE_TYPE=VM.Standard.E4.Flex}" + shapeConfig: + ocpus: "${OCI_NODE_MACHINE_TYPE_OCPUS=1}" + vnicAttachments: + - displayName: "CalicoNic" + nicIndex: 1 # second nic must be used for hyper-v + metadata: + ssh_authorized_keys: "${OCI_SSH_KEY}" + isPvEncryptionInTransitEnabled: ${OCI_NODE_PV_TRANSIT_ENCRYPTION=true} +--- +apiVersion: bootstrap.cluster.x-k8s.io/v1alpha4 +kind: KubeadmConfigTemplate +metadata: + name: "${CLUSTER_NAME}-md-win" +spec: + template: + spec: + joinConfiguration: + nodeRegistration: + criSocket: npipe:////./pipe/containerd-containerd + kubeletExtraArgs: + cloud-provider: external + provider-id: oci://{{ ds.meta_data["instance_id"] }} + feature-gates: WindowsHostProcessContainers=true + v: "2" + windows-priorityclass: ABOVE_NORMAL_PRIORITY_CLASS + name: '{{ ds.meta_data["local_hostname"] }}' + preKubeadmCommands: + - powershell C:\Windows\Setup\Scripts\enable_second_nic.ps1 + - powershell C:\Users\opc\attach_secondary_vnic.ps1 > C:\Users\opc\attach_secondary_vnic_log.txt +--- +apiVersion: cluster.x-k8s.io/v1beta1 +kind: MachineDeployment +metadata: + name: "${CLUSTER_NAME}-md-win" +spec: + clusterName: "${CLUSTER_NAME}" + replicas: ${NODE_MACHINE_COUNT} + selector: + matchLabels: + template: + spec: + clusterName: "${CLUSTER_NAME}" + version: "${KUBERNETES_VERSION}" + bootstrap: + configRef: + name: "${CLUSTER_NAME}-md-win" + apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 + kind: KubeadmConfigTemplate + infrastructureRef: + name: "${CLUSTER_NAME}-md-win" + apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 + kind: OCIMachineTemplate \ No newline at end of file diff --git a/test/e2e/cluster_test.go b/test/e2e/cluster_test.go index 37d376c0..1992f6fa 100644 --- a/test/e2e/cluster_test.go +++ b/test/e2e/cluster_test.go @@ -199,6 +199,29 @@ var _ = Describe("Workload cluster creation", func() { validateOLImage(namespace.Name, clusterName) }) + It("Windows - With 1 Linux control-plane nodes and with 1 Windows worker nodes using Calico CNI", func() { + clusterName = getClusterName(clusterNamePrefix, "windows-calico") + clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{ + ClusterProxy: bootstrapClusterProxy, + ConfigCluster: clusterctl.ConfigClusterInput{ + LogFolder: filepath.Join(artifactFolder, "clusters", bootstrapClusterProxy.GetName()), + ClusterctlConfigPath: clusterctlConfigPath, + KubeconfigPath: bootstrapClusterProxy.GetKubeconfigPath(), + InfrastructureProvider: clusterctl.DefaultInfrastructureProvider, + Flavor: "windows-calico", + Namespace: namespace.Name, + ClusterName: clusterName, + KubernetesVersion: e2eConfig.GetVariable(capi_e2e.KubernetesVersion), + ControlPlaneMachineCount: pointer.Int64Ptr(1), + WorkerMachineCount: pointer.Int64Ptr(1), + }, + WaitForClusterIntervals: e2eConfig.GetIntervals(specName, "wait-cluster"), + WaitForControlPlaneIntervals: e2eConfig.GetIntervals(specName, "wait-control-plane"), + WaitForMachineDeployments: e2eConfig.GetIntervals(specName, "wait-windows-worker-nodes"), + }, result) + validateWindowsImage(namespace.Name, clusterName) + }) + It("Cloud Provider OCI testing [PRBlocking]", func() { clusterName = getClusterName(clusterNamePrefix, "ccm-testing") clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{ @@ -960,6 +983,33 @@ func validateOLImage(nameSpace string, clusterName string) { } } +func validateWindowsImage(nameSpace string, clusterName string) { + lister := bootstrapClusterProxy.GetClient() + inClustersNamespaceListOption := client.InNamespace(nameSpace) + matchClusterListOption := client.MatchingLabels{ + clusterv1.ClusterLabelName: clusterName, + } + + machineList := &clusterv1.MachineList{} + Expect(lister.List(context.Background(), machineList, inClustersNamespaceListOption, matchClusterListOption)). + To(Succeed(), "Couldn't list machines for the cluster %q", clusterName) + + Expect(len(machineList.Items)).To(Equal(2)) + for _, machine := range machineList.Items { + if machine.Labels["os"] == "windows" { + instanceOcid := strings.Split(*machine.Spec.ProviderID, "//")[1] + Log(fmt.Sprintf("Instance OCID is %s", instanceOcid)) + resp, err := computeClient.GetInstance(context.Background(), core.GetInstanceRequest{ + InstanceId: common.String(instanceOcid), + }) + Expect(err).NotTo(HaveOccurred()) + instanceSourceDetails, ok := resp.SourceDetails.(core.InstanceSourceViaImageDetails) + Expect(ok).To(BeTrue()) + Expect(*instanceSourceDetails.ImageId).To(Equal(os.Getenv("OCI_WINDOWS_IMAGE_ID"))) + } + } +} + func getClusterName(prefix, specName string) string { clusterName := os.Getenv("CLUSTER_NAME") if clusterName == "" { diff --git a/test/e2e/config/e2e_conf.yaml b/test/e2e/config/e2e_conf.yaml index 145cabfb..bfb71e53 100644 --- a/test/e2e/config/e2e_conf.yaml +++ b/test/e2e/config/e2e_conf.yaml @@ -71,6 +71,7 @@ providers: - 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/cluster-template-windows-calico.yaml" - sourcePath: "../data/infrastructure-oci/v1beta1/metadata.yaml" variables: @@ -109,6 +110,7 @@ intervals: default/wait-cluster: ["30m", "10s"] default/wait-control-plane: ["30m", "10s"] default/wait-worker-nodes: ["30m", "10s"] + default/wait-windows-worker-nodes: ["60m", "30s"] default/wait-cluster-bare-metal: [ "60m", "10s" ] default/wait-control-plane-bare-metal: [ "60m", "10s" ] default/wait-worker-nodes-bare-metal: [ "60m", "10s" ] diff --git a/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/kustomization.yaml b/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/kustomization.yaml new file mode 100644 index 00000000..b0e2d842 --- /dev/null +++ b/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/kustomization.yaml @@ -0,0 +1,7 @@ +bases: +- ../bases/cluster.yaml +- ../bases/md.yaml +- ../bases/crs.yaml +- ../bases/ccm.yaml +patchesStrategicMerge: + - ./md.yaml \ No newline at end of file diff --git a/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/md.yaml b/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/md.yaml new file mode 100644 index 00000000..9f291624 --- /dev/null +++ b/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-windows-calico/md.yaml @@ -0,0 +1,46 @@ +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: OCIMachineTemplate +metadata: + name: "${CLUSTER_NAME}-md-0" + labels: + os: windows +spec: + template: + spec: + imageId: "${OCI_WINDOWS_IMAGE_ID}" + shape: "BM.Standard.E4.128" + shapeConfig: + ocpus: "128" + vnicAttachments: + - displayName: "CalicoNic" + nicIndex: 1 # second nic must be used for hyper-v + isPvEncryptionInTransitEnabled: ${OCI_NODE_PV_TRANSIT_ENCRYPTION=false} +--- +apiVersion: bootstrap.cluster.x-k8s.io/v1alpha4 +kind: KubeadmConfigTemplate +metadata: + name: "${CLUSTER_NAME}-md-0" +spec: + template: + spec: + joinConfiguration: + nodeRegistration: + criSocket: npipe:////./pipe/containerd-containerd + kubeletExtraArgs: + provider-id: oci://{{ ds.meta_data["instance_id"] }} + feature-gates: WindowsHostProcessContainers=true + v: "2" + windows-priorityclass: ABOVE_NORMAL_PRIORITY_CLASS + name: '{{ ds.meta_data["local_hostname"] }}' + preKubeadmCommands: + - powershell C:\Windows\Setup\Scripts\enable_second_nic.ps1 + - powershell C:\Users\opc\attach_secondary_vnic.ps1 > C:\Users\opc\attach_secondary_vnic_log.txt +--- +apiVersion: cluster.x-k8s.io/v1beta1 +kind: MachineDeployment +metadata: + name: "${CLUSTER_NAME}-md-0" +spec: + template: + spec: + failureDomain: "2" \ No newline at end of file