Skip to content
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

fix(IpRange): change GCP IpRange naming strategy #371

Merged
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
7 changes: 7 additions & 0 deletions api/cloud-control/v1beta1/iprange_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ type IpRangeStatus struct {
// Operation Identifier to track the Hyperscaler Operation
// +optional
OpIdentifier string `json:"opIdentifier,omitempty"`

// Id to track the Hyperscaler IpRange identifier
// +optional
Id string `json:"id,omitempty"`
}

type IpRangeSubnets []IpRangeSubnet
Expand Down Expand Up @@ -148,6 +152,9 @@ func (in IpRangeSubnets) SubnetByZone(zone string) *IpRangeSubnet {

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
// +kubebuilder:printcolumn:name="Cidr",type="string",JSONPath=".status.cidr"
// +kubebuilder:printcolumn:name="State",type="string",JSONPath=".status.state"

// IpRange is the Schema for the ipranges API
type IpRange struct {
Expand Down
15 changes: 14 additions & 1 deletion config/crd/bases/cloud-control.kyma-project.io_ipranges.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,17 @@ spec:
singular: iprange
scope: Namespaced
versions:
- name: v1beta1
- additionalPrinterColumns:
- jsonPath: .metadata.creationTimestamp
name: Age
type: date
- jsonPath: .status.cidr
name: Cidr
type: string
- jsonPath: .status.state
name: State
type: string
name: v1beta1
schema:
openAPIV3Schema:
description: IpRange is the Schema for the ipranges API
Expand Down Expand Up @@ -163,6 +173,9 @@ spec:
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
id:
description: Id to track the Hyperscaler IpRange identifier
type: string
opIdentifier:
description: Operation Identifier to track the Hyperscaler Operation
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,17 @@ spec:
singular: iprange
scope: Namespaced
versions:
- name: v1beta1
- additionalPrinterColumns:
- jsonPath: .metadata.creationTimestamp
name: Age
type: date
- jsonPath: .status.cidr
name: Cidr
type: string
- jsonPath: .status.state
name: State
type: string
name: v1beta1
schema:
openAPIV3Schema:
description: IpRange is the Schema for the ipranges API
Expand Down Expand Up @@ -163,6 +173,9 @@ spec:
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
id:
description: Id to track the Hyperscaler IpRange identifier
type: string
opIdentifier:
description: Operation Identifier to track the Hyperscaler Operation
type: string
Expand Down
9 changes: 5 additions & 4 deletions pkg/kcp/provider/gcp/iprange/v2/compareStates.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ func compareStates(ctx context.Context, st composed.State) (error, context.Conte
//Check and see whether the desiredState == actualState
deleting := !state.Obj().GetDeletionTimestamp().IsZero()
gcpOptions := state.ObjAsIpRange().Spec.Options.Gcp
ipRange := state.ObjAsIpRange()

state.addressOp = client.NONE
state.connectionOp = client.NONE
Expand Down Expand Up @@ -56,15 +55,17 @@ func compareStates(ctx context.Context, st composed.State) (error, context.Conte
//Check whether the IPRange is created for PSA.
//If yes, identify the operation to be performed.
//If no, serviceConnection object is not needed, so ignore.
if gcpOptions == nil || gcpOptions.Purpose == v1beta1.GcpPurposePSA {
if state.address == nil {
state.connectionOp = client.NONE
} else if gcpOptions == nil || gcpOptions.Purpose == v1beta1.GcpPurposePSA {
if state.serviceConnection == nil {
//If serviceConnection doesn't exist, add it.
state.connectionOp = client.ADD
state.ipRanges = []string{ipRange.Spec.RemoteRef.Name}
state.ipRanges = []string{state.address.Name}
} else if index := state.doesConnectionIncludeRange(); index < 0 {
//If connection exists, but the ipRange is not part of it, include it.
state.connectionOp = client.MODIFY
state.ipRanges = append(state.serviceConnection.ReservedPeeringRanges, ipRange.Spec.RemoteRef.Name)
state.ipRanges = append(state.serviceConnection.ReservedPeeringRanges, state.address.Name)
}
}

Expand Down
26 changes: 13 additions & 13 deletions pkg/kcp/provider/gcp/iprange/v2/compareStates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (suite *compareStatesSuite) TestWhenDeletingAndAddressExistsAndNoConnection
//Set Address
address := &compute.Address{
Address: ipAddr,
Name: gcpIpRange.Spec.RemoteRef.Name,
Name: GetIpRangeName(gcpIpRange.Name),
}
state.address = address

Expand Down Expand Up @@ -130,7 +130,7 @@ func (suite *compareStatesSuite) TestWhenDeletingAndNoAddressExistsAndConnection

//Set Connection
connection := &servicenetworking.Connection{
ReservedPeeringRanges: []string{ipRange.Spec.RemoteRef.Name},
ReservedPeeringRanges: []string{GetIpRangeName(ipRange.Name)},
}
state.serviceConnection = connection

Expand All @@ -140,10 +140,10 @@ func (suite *compareStatesSuite) TestWhenDeletingAndNoAddressExistsAndConnection
assert.Nil(suite.T(), resCtx)

//Validate state attributes
assert.Equal(suite.T(), client.DeletePsaConnection, state.curState)
assert.Equal(suite.T(), client.DELETE, state.connectionOp)
assert.Equal(suite.T(), client.Deleted, state.curState)
assert.Equal(suite.T(), client.NONE, state.connectionOp)
assert.Equal(suite.T(), client.NONE, state.addressOp)
assert.False(suite.T(), state.inSync)
assert.True(suite.T(), state.inSync)
}

func (suite *compareStatesSuite) TestWhenDeletingAndBothAddressAndConnectionExists() {
Expand Down Expand Up @@ -174,13 +174,13 @@ func (suite *compareStatesSuite) TestWhenDeletingAndBothAddressAndConnectionExis
//Set Address
address := &compute.Address{
Address: ipAddr,
Name: gcpIpRange.Spec.RemoteRef.Name,
Name: GetIpRangeName(gcpIpRange.Name),
}
state.address = address

//Set Connection
connection := &servicenetworking.Connection{
ReservedPeeringRanges: []string{ipRange.Spec.RemoteRef.Name, "test"},
ReservedPeeringRanges: []string{GetIpRangeName(ipRange.Name), "test"},
}
state.serviceConnection = connection

Expand Down Expand Up @@ -225,7 +225,7 @@ func (suite *compareStatesSuite) TestWhenNotDeleting_NoAddressOrConnectionExists

//Validate state attributes
assert.Equal(suite.T(), client.SyncAddress, state.curState)
assert.Equal(suite.T(), client.ADD, state.connectionOp)
assert.Equal(suite.T(), client.NONE, state.connectionOp)
assert.Equal(suite.T(), client.ADD, state.addressOp)
assert.False(suite.T(), state.inSync)
}
Expand Down Expand Up @@ -256,7 +256,7 @@ func (suite *compareStatesSuite) TestWhenNotDeleting_AddressExistsAndNoConnectio
address := &compute.Address{
Address: ipAddr,
PrefixLength: int64(prefix),
Name: gcpIpRange.Spec.RemoteRef.Name,
Name: GetIpRangeName(gcpIpRange.Name),
Network: state.Scope().Spec.Scope.Gcp.VpcNetwork,
}
state.address = address
Expand Down Expand Up @@ -301,7 +301,7 @@ func (suite *compareStatesSuite) TestWhenNotDeleting_AddressNotMatches() {
address := &compute.Address{
Address: ipAddr,
PrefixLength: int64(prefix),
Name: gcpIpRange.Spec.RemoteRef.Name,
Name: GetIpRangeName(gcpIpRange.Name),
}
state.address = address
state.ipAddress = ipAddr
Expand Down Expand Up @@ -345,7 +345,7 @@ func (suite *compareStatesSuite) TestWhenNotDeleting_AddressExistsAndConnectionN
address := &compute.Address{
Address: ipAddr,
PrefixLength: int64(prefix),
Name: gcpIpRange.Spec.RemoteRef.Name,
Name: GetIpRangeName(gcpIpRange.Name),
Network: state.Scope().Spec.Scope.Gcp.VpcNetwork,
}
state.address = address
Expand Down Expand Up @@ -396,7 +396,7 @@ func (suite *compareStatesSuite) TestWhenNotDeleting_BothAddressAndConnectionExi
address := &compute.Address{
Address: ipAddr,
PrefixLength: int64(prefix),
Name: gcpIpRange.Spec.RemoteRef.Name,
Name: GetIpRangeName(gcpIpRange.Name),
Network: state.Scope().Spec.Scope.Gcp.VpcNetwork,
}
state.address = address
Expand All @@ -405,7 +405,7 @@ func (suite *compareStatesSuite) TestWhenNotDeleting_BothAddressAndConnectionExi

//Set Connection
connection := &servicenetworking.Connection{
ReservedPeeringRanges: []string{gcpIpRange.Spec.RemoteRef.Name},
ReservedPeeringRanges: []string{GetIpRangeName(gcpIpRange.Name)},
}
state.serviceConnection = connection

Expand Down
75 changes: 55 additions & 20 deletions pkg/kcp/provider/gcp/iprange/v2/loadAddress.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,72 @@ package v2
import (
"context"
"errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"fmt"
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
"github.com/kyma-project/cloud-manager/pkg/composed"
"google.golang.org/api/googleapi"
gcpmeta "github.com/kyma-project/cloud-manager/pkg/kcp/provider/gcp/meta"
)

func loadAddress(ctx context.Context, st composed.State) (error, context.Context) {
state := st.(*State)
logger := composed.LoggerFromCtx(ctx)

ipRange := state.ObjAsIpRange()
logger.WithValues("ipRange :", ipRange.Name).Info("Loading GCP Address")

//Get from GCP.
gcpScope := state.Scope().Spec.Scope.Gcp
project := gcpScope.Project
vpc := gcpScope.VpcNetwork
name := ipRange.Spec.RemoteRef.Name
vpcName := gcpScope.VpcNetwork
remoteName := GetIpRangeName(ipRange.GetName())
remoteFallbackName := ipRange.Spec.RemoteRef.Name
logger = logger.WithValues(
"ipRange", ipRange.Name,
"ipRangeRemoteName", remoteName,
"ipRangeRemoteFallbackName", remoteFallbackName,
)

addr, err := state.computeClient.GetIpRange(ctx, project, name)
if err != nil {
logger.Info("Loading GCP Address (V2)")

addr, err := state.computeClient.GetIpRange(ctx, project, remoteName)

if gcpmeta.IsNotFound(err) {
// fallback to old name (backwards compatibility)
logger.Info("New IpRange not found, checking the old name")
fallbackAddr, err2 := state.computeClient.GetIpRange(ctx, project, remoteFallbackName)

if gcpmeta.IsNotFound(err2) {
logger.Info("Fallback IpRange name not found, proceeding")
return nil, nil
}

var e *googleapi.Error
if ok := errors.As(err, &e); ok {
if e.Code == 404 {
state.address = nil
return nil, nil
}
if err2 != nil {
logger.Error(err2, "Error getting fallback ipRange Addresses from GCP")
return composed.UpdateStatus(ipRange).
SetExclusiveConditions(metav1.Condition{
Type: v1beta1.ConditionTypeError,
Status: metav1.ConditionTrue,
Reason: v1beta1.ReasonGcpError,
Message: "Error getting fallback ipRange Addresses from GCP",
}).
SuccessError(composed.StopWithRequeue).
SuccessLogMsg("Updated condition for failed IpRange fetching").
Run(ctx, state)
}

if !strings.HasSuffix(fallbackAddr.Network, fmt.Sprintf("/%s", vpcName)) {
logger.Info("Target fallback ipRange doesnt belong to this VPC, skipping")
return nil, nil
}

state.address = fallbackAddr
return nil, nil
}

if err != nil {
logger.Error(err, "Error getting fallback ipRange Addresses from GCP")
return composed.UpdateStatus(ipRange).
SetExclusiveConditions(metav1.Condition{
Type: v1beta1.ConditionTypeError,
Expand All @@ -43,23 +77,24 @@ func loadAddress(ctx context.Context, st composed.State) (error, context.Context
Message: "Error getting Addresses from GCP",
}).
SuccessError(composed.StopWithRequeue).
SuccessLogMsg("Error getting Addresses from GCP").
SuccessLogMsg("Updated condition for failed IpRange fetching").
Run(ctx, state)
}

//Check whether the IPRange is in the same VPC as that of the SKR.
if !strings.HasSuffix(addr.Network, vpc) {
if !strings.HasSuffix(addr.Network, fmt.Sprintf("/%s", vpcName)) {
logger.Error(errors.New("obtained IpRange belongs to another VPC"), "Obtained IpRange belongs to another VPC")
return composed.UpdateStatus(ipRange).
SetExclusiveConditions(metav1.Condition{
Type: v1beta1.ConditionTypeError,
Status: metav1.ConditionTrue,
Reason: v1beta1.ReasonGcpError,
Message: "IPRange with the same name exists in another VPC.",
Message: "Obtained IpRange belongs to another VPC.",
}).
SuccessError(composed.StopWithRequeue).
SuccessLogMsg("GCP - IPRange name conflict").
SuccessError(composed.StopAndForget).
SuccessLogMsg("Obtained IpRange belongs to another VPC").
Run(ctx, state)
}

state.address = addr

return nil, nil
Expand Down
Loading
Loading