Skip to content

Add support for multiple CIDR in vcn and fix dns label bug #372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -895,10 +895,16 @@ type VCN struct {
// VCN Name.
// +optional
Name string `json:"name"`

// VCN CIDR.
// +optional
// Deprecated, please use NetworkDetails.cidrs
CIDR string `json:"cidr,omitempty"`

// VCN CIDRs.
// +optional
CIDRS []string `json:"cidrs,omitempty"`

// ID of Nat Gateway.
// +optional
NatGatewayId *string `json:"natGatewayId,omitempty"`
Expand Down
2 changes: 2 additions & 0 deletions api/v1beta1/zz_generated.conversion.go

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

5 changes: 5 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

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

6 changes: 6 additions & 0 deletions api/v1beta2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -894,10 +894,16 @@ type VCN struct {
// VCN Name.
// +optional
Name string `json:"name"`

// VCN CIDR.
// +optional
// Deprecated, please use NetworkDetails.cidrs
CIDR string `json:"cidr,omitempty"`

// VCN CIDRs.
// +optional
CIDRS []string `json:"cidrs,omitempty"`

// Subnets is the configuration for subnets required in the VCN.
// +optional
// +listType=map
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

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

36 changes: 15 additions & 21 deletions cloud/scope/vcn_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
)

func (s *ClusterScope) ReconcileVCN(ctx context.Context) error {
desiredVCN := s.VCNSpec()
spec := s.OCIClusterAccessor.GetNetworkSpec().Vcn

var err error
vcn, err := s.GetVCN(ctx)
Expand All @@ -37,19 +37,19 @@ func (s *ClusterScope) ReconcileVCN(ctx context.Context) error {
}
if vcn != nil {
s.OCIClusterAccessor.GetNetworkSpec().Vcn.ID = vcn.Id
if s.IsVcnEquals(vcn, desiredVCN) {
if s.IsVcnEquals(vcn) {
s.Logger.Info("No Reconciliation Required for VCN", "vcn", s.getVcnId())
return nil
}
return s.UpdateVCN(ctx, desiredVCN)
return s.UpdateVCN(ctx, spec)
}
vcnId, err := s.CreateVCN(ctx, desiredVCN)
vcnId, err := s.CreateVCN(ctx, spec)
s.OCIClusterAccessor.GetNetworkSpec().Vcn.ID = vcnId
return err
}

func (s *ClusterScope) IsVcnEquals(actual *core.Vcn, desired infrastructurev1beta2.VCN) bool {
if *actual.DisplayName != desired.Name {
func (s *ClusterScope) IsVcnEquals(actual *core.Vcn) bool {
if *actual.DisplayName != s.GetVcnName() {
return false
}
return true
Expand All @@ -62,19 +62,13 @@ func (s *ClusterScope) GetVcnName() string {
return fmt.Sprintf("%s", s.OCIClusterAccessor.GetName())
}

func (s *ClusterScope) GetVcnCidr() string {
if s.OCIClusterAccessor.GetNetworkSpec().Vcn.CIDR != "" {
return s.OCIClusterAccessor.GetNetworkSpec().Vcn.CIDR
func (s *ClusterScope) GetVcnCidrs() []string {
if s.OCIClusterAccessor.GetNetworkSpec().Vcn.CIDRS != nil && len(s.OCIClusterAccessor.GetNetworkSpec().Vcn.CIDRS) > 0 {
return s.OCIClusterAccessor.GetNetworkSpec().Vcn.CIDRS
} else if s.OCIClusterAccessor.GetNetworkSpec().Vcn.CIDR != "" {
return []string{s.OCIClusterAccessor.GetNetworkSpec().Vcn.CIDR}
}
return VcnDefaultCidr
}

func (s *ClusterScope) VCNSpec() infrastructurev1beta2.VCN {
vcnSpec := infrastructurev1beta2.VCN{
Name: s.GetVcnName(),
CIDR: s.GetVcnCidr(),
}
return vcnSpec
return []string{VcnDefaultCidr}
}

func (s *ClusterScope) GetVCN(ctx context.Context) (*core.Vcn, error) {
Expand Down Expand Up @@ -112,7 +106,7 @@ func (s *ClusterScope) GetVCN(ctx context.Context) (*core.Vcn, error) {

func (s *ClusterScope) UpdateVCN(ctx context.Context, vcn infrastructurev1beta2.VCN) error {
updateVCNDetails := core.UpdateVcnDetails{
DisplayName: common.String(vcn.Name),
DisplayName: common.String(s.GetVcnName()),
}
vcnResponse, err := s.VCNClient.UpdateVcn(ctx, core.UpdateVcnRequest{
UpdateVcnDetails: updateVCNDetails,
Expand All @@ -129,8 +123,8 @@ func (s *ClusterScope) UpdateVCN(ctx context.Context, vcn infrastructurev1beta2.
func (s *ClusterScope) CreateVCN(ctx context.Context, spec infrastructurev1beta2.VCN) (*string, error) {
vcnDetails := core.CreateVcnDetails{
CompartmentId: common.String(s.GetCompartmentId()),
DisplayName: common.String(spec.Name),
CidrBlocks: []string{spec.CIDR},
DisplayName: common.String(s.GetVcnName()),
CidrBlocks: s.GetVcnCidrs(),
FreeformTags: s.GetFreeFormTags(),
DefinedTags: s.GetDefinedTags(),
DnsLabel: spec.DnsLabel,
Expand Down
47 changes: 37 additions & 10 deletions cloud/scope/vcn_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,23 @@ func TestClusterScope_CreateVCN(t *testing.T) {
vcnClient := mock_vcn.NewMockClient(mockCtrl)

vcnClient.EXPECT().CreateVcn(gomock.Any(), Eq(func(request interface{}) error {
return vcnMatcher(request, "normal", common.String("label"))
return vcnMatcher(request, "normal", common.String("label"), []string{"test-cidr"})
})).
Return(core.CreateVcnResponse{
Vcn: core.Vcn{
Id: common.String("normal_id"),
},
}, nil)
vcnClient.EXPECT().CreateVcn(gomock.Any(), Eq(func(request interface{}) error {
return vcnMatcher(request, "error", nil)
return vcnMatcher(request, "normal", common.String("label"), []string{"test-cidr1", "test-cidr2"})
})).
Return(core.CreateVcnResponse{
Vcn: core.Vcn{
Id: common.String("normal_id"),
},
}, nil)
vcnClient.EXPECT().CreateVcn(gomock.Any(), Eq(func(request interface{}) error {
return vcnMatcher(request, "error", nil, []string{VcnDefaultCidr})
})).
Return(core.CreateVcnResponse{}, errors.New("some error"))

Expand All @@ -65,6 +73,21 @@ func TestClusterScope_CreateVCN(t *testing.T) {
Vcn: infrastructurev1beta2.VCN{
Name: "normal",
DnsLabel: common.String("label"),
CIDR: "test-cidr",
},
},
},
want: common.String("normal_id"),
wantErr: false,
},
{
name: "create vcn is successful, multiple cidrs",
spec: infrastructurev1beta2.OCIClusterSpec{
NetworkSpec: infrastructurev1beta2.NetworkSpec{
Vcn: infrastructurev1beta2.VCN{
Name: "normal",
DnsLabel: common.String("label"),
CIDRS: []string{"test-cidr1", "test-cidr2"},
},
},
},
Expand Down Expand Up @@ -392,11 +415,11 @@ func TestClusterScope_GetVcnCidr(t *testing.T) {
tests := []struct {
name string
spec infrastructurev1beta2.OCIClusterSpec
want string
want []string
}{
{
name: "cidr not present",
want: VcnDefaultCidr,
want: []string{VcnDefaultCidr},
},
{
name: "cidr present",
Expand All @@ -407,7 +430,7 @@ func TestClusterScope_GetVcnCidr(t *testing.T) {
},
},
},
want: "foo",
want: []string{"foo"},
},
}
l := log.FromContext(context.Background())
Expand All @@ -426,7 +449,7 @@ func TestClusterScope_GetVcnCidr(t *testing.T) {
OCIClusterAccessor: ociClusterAccessor,
Logger: &l,
}
if got := s.GetVcnCidr(); got != tt.want {
if got := s.GetVcnCidrs(); !reflect.DeepEqual(got, tt.want) {
t.Errorf("GetVcnCidr() = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -519,7 +542,7 @@ func TestClusterScope_IsVcnEquals(t *testing.T) {
},
Logger: &l,
}
if got := s.IsVcnEquals(tt.actual, tt.desired); got != tt.want {
if got := s.IsVcnEquals(tt.actual); got != tt.want {
t.Errorf("IsVcnEquals() = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -603,7 +626,7 @@ func TestClusterScope_ReconcileVCN(t *testing.T) {
}}, nil)

vcnClient.EXPECT().CreateVcn(gomock.Any(), Eq(func(request interface{}) error {
return vcnMatcher(request, "not_found", nil)
return vcnMatcher(request, "not_found", common.String("label"), []string{VcnDefaultCidr})
})).
Return(core.CreateVcnResponse{
Vcn: core.Vcn{
Expand Down Expand Up @@ -664,7 +687,8 @@ func TestClusterScope_ReconcileVCN(t *testing.T) {
CompartmentId: "bar",
NetworkSpec: infrastructurev1beta2.NetworkSpec{
Vcn: infrastructurev1beta2.VCN{
Name: "not_found",
Name: "not_found",
DnsLabel: common.String("label"),
},
},
},
Expand Down Expand Up @@ -706,7 +730,7 @@ func TestClusterScope_ReconcileVCN(t *testing.T) {
}
}

func vcnMatcher(request interface{}, displayName string, dnsLabel *string) error {
func vcnMatcher(request interface{}, displayName string, dnsLabel *string, cidrs []string) error {
r, ok := request.(core.CreateVcnRequest)
if !ok {
return errors.New("expecting CreateVcnRequest type")
Expand All @@ -717,5 +741,8 @@ func vcnMatcher(request interface{}, displayName string, dnsLabel *string) error
if !reflect.DeepEqual(r.CreateVcnDetails.DnsLabel, dnsLabel) {
return errors.New(fmt.Sprintf("expecting DnsLabel as %v", dnsLabel))
}
if !reflect.DeepEqual(r.CreateVcnDetails.CidrBlocks, cidrs) {
return errors.New(fmt.Sprintf("expecting cidrblocks as %v, actual %v", cidrs, r.CreateVcnDetails.CidrBlocks))
}
return nil
}
14 changes: 12 additions & 2 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_ociclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the VCN, used
in conjunction with the VNIC's hostname and subnet's DNS
Expand Down Expand Up @@ -1293,8 +1298,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the VCN, used
in conjunction with the VNIC's hostname and subnet's DNS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the
VCN, used in conjunction with the VNIC's hostname
Expand Down Expand Up @@ -1353,8 +1358,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the
VCN, used in conjunction with the VNIC's hostname
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the VCN, used
in conjunction with the VNIC's hostname and subnet's DNS
Expand Down Expand Up @@ -1299,8 +1304,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the VCN, used
in conjunction with the VNIC's hostname and subnet's DNS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the
VCN, used in conjunction with the VNIC's hostname
Expand Down Expand Up @@ -1363,8 +1368,13 @@ spec:
description: VCN configuration.
properties:
cidr:
description: VCN CIDR.
description: VCN CIDR. Deprecated, please use NetworkDetails.cidrs
type: string
cidrs:
description: VCN CIDRs.
items:
type: string
type: array
dnsLabel:
description: DnsLabel specifies a DNS label for the
VCN, used in conjunction with the VNIC's hostname
Expand Down
Loading