Merge branch 'main' into peering/aws/status-state

internal/controller/cloud-control/vpcpeering_azure_test.go

@@ -415,6 +415,20 @@ var _ = Describe("Feature: KCP VpcPeering", func() {
 				).Should(Succeed())
 		})
 
+		// Waits for the reconcilers to create Azure peerings
+		By("And Then remote Azure VPC Peering is created", func() {
+			Eventually(func() error {
+				p, err := azureMockRemote.GetPeering(infra.Ctx(), remoteResourceGroup, remoteVnetName, remotePeeringName)
+				if err != nil {
+					return err
+				}
+				if p == nil {
+					return errors.New("nil peering received")
+				}
+				return nil
+			}).Should(Succeed())
+		})
+
 		// Ready ==========================================================
 
 		By("When Azure VPC Peering is Connected", func() {

internal/controller/cloud-control/vpcpeering_gcp_test.go

@@ -2,6 +2,7 @@ package cloudcontrol
 
 import (
 	pb "cloud.google.com/go/compute/apiv1/computepb"
+	"errors"
 	cloudcontrolv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
 	networkPkg "github.com/kyma-project/cloud-manager/pkg/kcp/network"
 	scopePkg "github.com/kyma-project/cloud-manager/pkg/kcp/scope"
@@ -184,6 +185,153 @@ var _ = Describe("Feature: KCP VpcPeering", func() {
 				Should(Succeed(), "Error deleting VPC Peering")
 		})
 
+		By("Then VpcPeering does not exist", func() {
+			Eventually(IsDeleted).
+				WithArguments(infra.Ctx(), infra.KCP().Client(), vpcpeering).
+				Should(Succeed(), "VPC Peering was not deleted")
+		})
+	})
+
+	It("Scenario: KCP GCP VpcPeering can be deleted due to issues on the remote network", func() {
+		const (
+			kymaName           = "ec697362-8f63-4423-b34f-8a99c0460d46"
+			kymaNetworkName    = kymaName + "--kyma"
+			kymaProject        = "kyma-project"
+			kymaVpc            = "shoot-12345-abc"
+			remoteNetworkName  = "0ab0eca3-3094-4842-9834-7492aaa0639d"
+			remotePeeringName  = "peering-with-permission-error"
+			remoteVpc          = "remote-vpc"
+			remoteProject      = "remote-project"
+			remoteRefNamespace = "kcp-system"
+			remoteRefName      = "skr-gcp-vpcpeering"
+			importCustomRoutes = false
+		)
+
+		scope := &cloudcontrolv1beta1.Scope{}
+
+		By("Given Scope exists", func() {
+			scopePkg.Ignore.AddName(kymaName)
+
+			Eventually(CreateScopeGcp).
+				WithArguments(infra.Ctx(), infra, scope, WithName(kymaName)).
+				Should(Succeed())
+		})
+
+		// and Given the Kyma network object exists in KCP
+		kymaNetwork := &cloudcontrolv1beta1.Network{
+			Spec: cloudcontrolv1beta1.NetworkSpec{
+				Network: cloudcontrolv1beta1.NetworkInfo{
+					Reference: &cloudcontrolv1beta1.NetworkReference{
+						Gcp: &cloudcontrolv1beta1.GcpNetworkReference{
+							GcpProject:  kymaProject,
+							NetworkName: kymaVpc,
+						},
+					},
+				},
+				Type: cloudcontrolv1beta1.NetworkTypeKyma,
+			},
+		}
+
+		By("And Given Kyma Network exists in KCP", func() {
+			// Tell Scope reconciler to ignore this kymaName
+			networkPkg.Ignore.AddName(kymaNetworkName)
+
+			Eventually(CreateObj).
+				WithArguments(infra.Ctx(), infra.KCP().Client(), kymaNetwork, WithName(kymaNetworkName), WithScope(scope.Name)).
+				Should(Succeed())
+		})
+
+		// and Given the remote network object exists in KCP
+		remoteNetwork := &cloudcontrolv1beta1.Network{
+			Spec: cloudcontrolv1beta1.NetworkSpec{
+				Network: cloudcontrolv1beta1.NetworkInfo{
+					Reference: &cloudcontrolv1beta1.NetworkReference{
+						Gcp: &cloudcontrolv1beta1.GcpNetworkReference{
+							GcpProject:  remoteProject,
+							NetworkName: remoteVpc,
+						},
+					},
+				},
+				Type: cloudcontrolv1beta1.NetworkTypeExternal,
+			},
+		}
+
+		By("And Given Remote Network exists in KCP", func() {
+			// Tell Scope reconciler to ignore this kymaName
+			networkPkg.Ignore.AddName(remoteNetworkName)
+
+			Eventually(CreateObj).
+				WithArguments(infra.Ctx(), infra.KCP().Client(), remoteNetwork, WithName(remoteNetworkName), WithScope(scope.Name), WithState("Ready")).
+				Should(Succeed())
+		})
+
+		By("And Given KCP KymaNetwork is Ready", func() {
+			Eventually(UpdateStatus).
+				WithArguments(infra.Ctx(),
+					infra.KCP().Client(),
+					kymaNetwork,
+					WithState("Ready"),
+					WithConditions(KcpReadyCondition())).
+				Should(Succeed())
+		})
+
+		By("And Given KCP RemoteNetwork is Ready", func() {
+			Eventually(UpdateStatus).
+				WithArguments(infra.Ctx(),
+					infra.KCP().Client(),
+					remoteNetwork,
+					WithState("Ready"),
+					WithConditions(KcpReadyCondition())).
+				Should(Succeed())
+		})
+
+		vpcpeering := &cloudcontrolv1beta1.VpcPeering{
+			Spec: cloudcontrolv1beta1.VpcPeeringSpec{
+				Details: &cloudcontrolv1beta1.VpcPeeringDetails{
+					LocalNetwork: klog.ObjectRef{
+						Name:      kymaNetwork.Name,
+						Namespace: kymaNetwork.Namespace,
+					},
+					RemoteNetwork: klog.ObjectRef{
+						Name:      remoteNetwork.Name,
+						Namespace: remoteNetwork.Namespace,
+					},
+					PeeringName:        remotePeeringName,
+					LocalPeeringName:   "cm-" + remoteNetworkName,
+					ImportCustomRoutes: importCustomRoutes,
+				},
+			},
+		}
+
+		By("And Given there is no permission to read remote network tags", func() {
+			infra.GcpMock().SetMockVpcPeeringError(remoteProject, remoteVpc, errors.New("permission denied"))
+		})
+
+		By("And Given KCP VpcPeering is created", func() {
+			Eventually(CreateObj).
+				WithArguments(infra.Ctx(), infra.KCP().Client(), vpcpeering,
+					WithName(remoteNetworkName),
+					WithRemoteRef(remoteRefName),
+					WithScope(kymaName),
+				).
+				Should(Succeed())
+		})
+
+		By("Then KCP VpcPeering has Error condition", func() {
+			Eventually(LoadAndCheck).
+				WithArguments(infra.Ctx(), infra.KCP().Client(), vpcpeering,
+					NewObjActions(),
+					HavingConditionTrue(cloudcontrolv1beta1.ConditionTypeError),
+				).
+				Should(Succeed())
+		})
+
+		By("When KCP VpcPeering in error state is deleted", func() {
+			Eventually(Delete).
+				WithArguments(infra.Ctx(), infra.KCP().Client(), vpcpeering).
+				Should(Succeed(), "Error deleting VPC Peering")
+		})
+
 		By("Then VpcPeering does not exist", func() {
 			Eventually(IsDeleted).
 				WithArguments(infra.Ctx(), infra.KCP().Client(), vpcpeering).

pkg/kcp/provider/gcp/mock/vpcPeeringStore.go

@@ -12,13 +12,15 @@ type vpcPeeringEntry struct {
 	peering *pb.NetworkPeering
 }
 type vpcPeeringStore struct {
-	m     sync.Mutex
-	items map[string]*vpcPeeringEntry
+	m        sync.Mutex
+	items    map[string]*vpcPeeringEntry
+	errorMap map[string]error
 }
 
 type VpcPeeringMockClientUtils interface {
 	GetMockVpcPeering(project string, vpc string) *pb.NetworkPeering
 	SetMockVpcPeeringLifeCycleState(project string, vpc string, state pb.NetworkPeering_State)
+	SetMockVpcPeeringError(project string, vpc string, err error)
 }
 
 func getFullNetworkUrl(project, vpc string) string {
@@ -102,12 +104,20 @@ func (s *vpcPeeringStore) GetVpcPeering(ctx context.Context, remotePeeringName s
 	s.m.Lock()
 	defer s.m.Unlock()
 
+	network := getFullNetworkUrl(project, vpc)
+
+	if s.errorMap == nil {
+		s.errorMap = make(map[string]error)
+	}
+
+	if err, errorExists := s.errorMap[network]; errorExists {
+		return nil, err
+	}
+
 	if s.items == nil {
 		s.items = make(map[string]*vpcPeeringEntry)
 	}
 
-	network := getFullNetworkUrl(project, vpc)
-
 	_, peeringExists := s.items[network]
 	if !peeringExists {
 		return nil, nil
@@ -144,3 +154,10 @@ func (s *vpcPeeringStore) GetMockVpcPeering(project string, vpc string) *pb.Netw
 	}
 	return s.items[getFullNetworkUrl(project, vpc)].peering
 }
+
+func (s *vpcPeeringStore) SetMockVpcPeeringError(project string, vpc string, err error) {
+	if s.errorMap == nil {
+		s.errorMap = make(map[string]error)
+	}
+	s.errorMap[getFullNetworkUrl(project, vpc)] = err
+}

pkg/kcp/provider/gcp/vpcpeering/loadRemoteNetwork.go

@@ -31,6 +31,11 @@ func loadRemoteNetwork(ctx context.Context, st composed.State) (error, context.C
 	}
 
 	if apierrors.IsNotFound(err) {
+		if composed.IsMarkedForDeletion(state.ObjAsVpcPeering()) {
+			logger.Error(err, "[KCP GCP VPCPeering loadRemoteNetwork] GCP KCP VpcPeering Remote Network not found, proceeding with deletion")
+			return nil, nil
+		}
+
 		state.ObjAsVpcPeering().Status.State = string(cloudcontrolv1beta1.ErrorState)
 		return composed.PatchStatus(state.ObjAsVpcPeering()).
 			SetExclusiveConditions(metav1.Condition{

pkg/kcp/provider/gcp/vpcpeering/loadRemoteVpcPeering.go

@@ -22,6 +22,11 @@ func loadRemoteVpcPeering(ctx context.Context, st composed.State) (error, contex
 
 	remoteVpcPeering, err := state.client.GetVpcPeering(ctx, state.remotePeeringName, state.remoteNetwork.Spec.Network.Reference.Gcp.GcpProject, state.remoteNetwork.Spec.Network.Reference.Gcp.NetworkName)
 	if err != nil {
+		if composed.IsMarkedForDeletion(state.ObjAsVpcPeering()) {
+			logger.Info("[KCP GCP VPCPeering loadRemoteVPCPeering] GCP KCP VpcPeering Error fetching Remote Network, proceeding with deletion")
+			return nil, nil
+		}
+
 		state.ObjAsVpcPeering().Status.State = v1beta1.VirtualNetworkPeeringStateDisconnected
 		logger.Error(err, "Error loading Remote VpcPeering")
 		meta.SetStatusCondition(state.ObjAsVpcPeering().Conditions(), metav1.Condition{

pkg/skr/gcpvpcpeering/deleteKcpRemoteNetwork.go

@@ -10,23 +10,15 @@ func deleteKcpRemoteNetwork(ctx context.Context, st composed.State) (error, cont
 	state := st.(*State)
 	logger := composed.LoggerFromCtx(ctx)
 
-	if !composed.MarkedForDeletionPredicate(ctx, state) {
-		return nil, nil
-	}
-
 	if state.KcpRemoteNetwork == nil {
 		return nil, nil
 	}
 
 	if composed.IsMarkedForDeletion(state.KcpRemoteNetwork) {
+		logger.Info("[SKR GCP VPCPeering deleteKcpRemoteNetwork] KCP Remote Network is marked for deletion, will continue.")
 		return nil, nil
 	}
 
-	if state.KcpVpcPeering != nil {
-		logger.Info("[SKR GCP VPCPeering deleteKcpRemoteNetwork] Waiting for KCP VpcPeering deletion")
-		return composed.StopWithRequeueDelay(3 * util.Timing.T1000ms()), nil
-	}
-
 	logger.Info("[SKR GCP VPCPeering deleteKcpRemoteNetwork] Deleting GCP KCP Remote Network")
 
 	err := state.KcpCluster.K8sClient().Delete(ctx, state.KcpRemoteNetwork)

pkg/skr/gcpvpcpeering/deleteKcpVpcPeering.go

@@ -11,25 +11,21 @@ func deleteKcpVpcPeering(ctx context.Context, st composed.State) (error, context
 	state := st.(*State)
 	logger := composed.LoggerFromCtx(ctx)
 
-	if !composed.MarkedForDeletionPredicate(ctx, state) {
-		return nil, nil
-	}
-
 	if state.KcpVpcPeering == nil {
-		// VpcPeering on SKR is marked for deletion, but not found in KCP, so probably it is already deleted
 		return nil, nil
 	}
 
 	if composed.IsMarkedForDeletion(state.KcpVpcPeering) {
-		return nil, nil
+		logger.Info("[SKR GCP VPCPeering deleteKcpVpcPeering] KCP VpcPeering is marked for deletion, re-queueing until it is deleted.")
+		return composed.StopWithRequeueDelay(util.Timing.T10000ms()), nil
 	}
 
-	logger.Info("Deleting KCP VpcPeering")
+	logger.Info("[SKR GCP VPCPeering deleteKcpVpcPeering] Deleting KCP VpcPeering")
 
 	err := state.KcpCluster.K8sClient().Delete(ctx, state.KcpVpcPeering)
 
 	if err != nil {
-		return composed.LogErrorAndReturn(err, "Error deleting KCP VpcPeering", composed.StopWithRequeue, ctx)
+		return composed.LogErrorAndReturn(err, "[SKR GCP VPCPeering deleteKcpVpcPeering] Error deleting KCP VpcPeering", composed.StopWithRequeue, ctx)
 	}
 
 	state.ObjAsGcpVpcPeering().Status.State = cloudcontrolv1beta1.VirtualNetworkPeeringStateDeleting