Skip to content

Commit

Permalink
resources phase explicit signaling
Browse files Browse the repository at this point in the history
allows peering-related resources (i.e. ResourceRequest and ResourceOffers) to have an explicit mechanism to indicate the desired state (Create os Delete) handled by the home cluster, and the observed state (Created od Deleted) handled by the remote cluster. In this way, we are able the handle a graceful deletion of these resources and to have a graceful peering tear down. The VirtualKublet can now have the time required for a completed clean-up of the remote cluster.
  • Loading branch information
aleoli committed Jul 5, 2021
1 parent 037fff9 commit 1c9f792
Show file tree
Hide file tree
Showing 16 changed files with 341 additions and 39 deletions.
6 changes: 5 additions & 1 deletion apis/discovery/v1alpha1/resourcerequest_types.go
Expand Up @@ -22,15 +22,20 @@ type ResourceRequestSpec struct {
ClusterIdentity ClusterIdentity `json:"clusterIdentity"`
// Local auth service address
AuthURL string `json:"authUrl"`
// WithdrawalTimestamp is set when a graceful deletion is requested by the user.
WithdrawalTimestamp *metav1.Time `json:"withdrawalTimestamp,omitempty"`
}

// ResourceRequestStatus defines the observed state of ResourceRequest.
type ResourceRequestStatus struct {
BroadcasterRef *object_references.DeploymentReference `json:"broadcasterRef,omitempty"`
AdvertisementStatus advtypes.AdvPhase `json:"advertisementStatus,omitempty"`
// OfferWithdrawalTimestamp is the withdrawal timestamp of the child ResourceOffer resource.
OfferWithdrawalTimestamp *metav1.Time `json:"offerWithdrawalTimestamp,omitempty"`
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status

// ResourceRequest is the Schema for the ResourceRequests API.
type ResourceRequest struct {
Expand All @@ -42,7 +47,6 @@ type ResourceRequest struct {
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status

// ResourceRequestList contains a list of ResourceRequest.
type ResourceRequestList struct {
Expand Down
10 changes: 9 additions & 1 deletion apis/discovery/v1alpha1/zz_generated.deepcopy.go

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

2 changes: 2 additions & 0 deletions apis/sharing/v1alpha1/resourceoffer_types.go
Expand Up @@ -23,6 +23,8 @@ type ResourceOfferSpec struct {
// TimeToLive is the time instant until this ResourceOffer will be valid.
// If not refreshed, an ResourceOffer will expire after 30 minutes.
TimeToLive metav1.Time `json:"timeToLive"`
// WithdrawalTimestamp is set when a graceful deletion is requested by the user.
WithdrawalTimestamp *metav1.Time `json:"withdrawalTimestamp,omitempty"`
}

// OfferPhase describes the phase of the ResourceOffer.
Expand Down
4 changes: 4 additions & 0 deletions apis/sharing/v1alpha1/zz_generated.deepcopy.go

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

12 changes: 12 additions & 0 deletions deployments/liqo/crds/discovery.liqo.io_resourcerequests.yaml
Expand Up @@ -52,6 +52,11 @@ spec:
required:
- clusterID
type: object
withdrawalTimestamp:
description: WithdrawalTimestamp is set when a graceful deletion is
requested by the user.
format: date-time
type: string
required:
- authUrl
- clusterIdentity
Expand All @@ -75,10 +80,17 @@ spec:
name must be unique.
type: string
type: object
offerWithdrawalTimestamp:
description: OfferWithdrawalTimestamp is the withdrawal timestamp
of the child ResourceOffer resource.
format: date-time
type: string
type: object
type: object
served: true
storage: true
subresources:
status: {}
status:
acceptedNames:
kind: ""
Expand Down
5 changes: 5 additions & 0 deletions deployments/liqo/crds/sharing.liqo.io_resourceoffers.yaml
Expand Up @@ -166,6 +166,11 @@ spec:
was created.
format: date-time
type: string
withdrawalTimestamp:
description: WithdrawalTimestamp is set when a graceful deletion is
requested by the user.
format: date-time
type: string
required:
- clusterId
- timeToLive
Expand Down
Expand Up @@ -167,7 +167,8 @@ func (r *ForeignClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
// ------ (3) peering/unpeering logic ------

// read the ForeignCluster status and ensure the peering state
switch r.getDesiredOutgoingPeeringState(&foreignCluster) {
phase := r.getDesiredOutgoingPeeringState(&foreignCluster)
switch phase {
case desiredPeeringPhasePeering:
if err = r.peerNamespaced(ctx, &foreignCluster); err != nil {
klog.Error(err)
Expand All @@ -178,6 +179,10 @@ func (r *ForeignClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
klog.Error(err)
return ctrl.Result{}, err
}
default:
err := fmt.Errorf("unknown phase %v", phase)
klog.Error(err)
return ctrl.Result{}, err
}

// ------ (4) update peering conditions ------
Expand Down Expand Up @@ -337,14 +342,39 @@ func (r *ForeignClusterReconciler) Unpeer(fc *discoveryv1alpha1.ForeignCluster,
// unpeerNamespaced disables the peering deleting the resources in the correct TenantControlNamespace.
func (r *ForeignClusterReconciler) unpeerNamespaced(ctx context.Context,
foreignCluster *discoveryv1alpha1.ForeignCluster) error {
// resource request has to be removed
err := r.deleteResourceRequest(ctx, foreignCluster)
if err != nil && !errors.IsNotFound(err) {
var resourceRequest discoveryv1alpha1.ResourceRequest
err := r.Client.Get(ctx, types.NamespacedName{
Namespace: foreignCluster.Status.TenantControlNamespace.Local,
Name: r.clusterID.GetClusterID(),
}, &resourceRequest)
if errors.IsNotFound(err) {
foreignCluster.Status.Outgoing.PeeringPhase = discoveryv1alpha1.PeeringPhaseNone
return nil
}
if err != nil {
klog.Error(err)
return err
}

foreignCluster.Status.Outgoing.PeeringPhase = discoveryv1alpha1.PeeringPhaseDisconnecting
if resourceRequest.Status.OfferWithdrawalTimestamp.IsZero() {
if resourceRequest.Spec.WithdrawalTimestamp.IsZero() {
now := metav1.Now()
resourceRequest.Spec.WithdrawalTimestamp = &now
}
err = r.Client.Update(ctx, &resourceRequest)
if err != nil && !errors.IsNotFound(err) {
klog.Error(err)
return err
}
foreignCluster.Status.Outgoing.PeeringPhase = discoveryv1alpha1.PeeringPhaseDisconnecting
} else {
err = r.deleteResourceRequest(ctx, foreignCluster)
if err != nil && !errors.IsNotFound(err) {
klog.Error(err)
return err
}
foreignCluster.Status.Outgoing.PeeringPhase = discoveryv1alpha1.PeeringPhaseNone
}
return nil
}

Expand Down Expand Up @@ -400,6 +430,12 @@ func getPeeringPhase(resourceRequestList *discoveryv1alpha1.ResourceRequestList)
case 0:
return discoveryv1alpha1.PeeringPhaseNone, nil
case 1:
resourceRequest := &resourceRequestList.Items[0]
desiredDelete := !resourceRequest.Spec.WithdrawalTimestamp.IsZero()
deleted := !resourceRequest.Status.OfferWithdrawalTimestamp.IsZero()
if desiredDelete && !deleted {
return discoveryv1alpha1.PeeringPhaseDisconnecting, nil
}
return discoveryv1alpha1.PeeringPhaseEstablished, nil
default:
err := fmt.Errorf("more than one resource request found")
Expand Down
Expand Up @@ -58,6 +58,8 @@ var _ = Describe("ForeignClusterOperator", func() {
mgr manager.Manager
ctx context.Context
cancel context.CancelFunc

now = metav1.Now()
)

BeforeEach(func() {
Expand Down Expand Up @@ -451,6 +453,15 @@ var _ = Describe("ForeignClusterOperator", func() {
Expect(ok).To(BeTrue())
Expect(rr).NotTo(BeNil())

// set the ResourceRequest status to created
obj, err = controller.crdClient.Resource("resourcerequests").Namespace(tenantNamespace.Name).UpdateStatus(rr.Name, rr, &metav1.UpdateOptions{})
Expect(err).To(BeNil())
Expect(obj).NotTo(BeNil())

rr, ok = obj.(*discoveryv1alpha1.ResourceRequest)
Expect(ok).To(BeTrue())
Expect(rr).NotTo(BeNil())

// disable the peering for that foreigncluster
err = controller.unpeerNamespaced(ctx, fc)
Expect(err).To(BeNil())
Expand All @@ -469,8 +480,37 @@ var _ = Describe("ForeignClusterOperator", func() {
Expect(rrs).NotTo(BeNil())

// check that the length of the resource request list is the expected one,
// and the resource request has been deleted in the correct namespace
// and the resource request has been set for deletion in the correct namespace
Expect(len(rrs.Items)).To(c.expectedPeeringLength)
if len(rrs.Items) > 0 {
Expect(rrs.Items[0].Spec.WithdrawalTimestamp.IsZero()).To(BeFalse())
rr = &rrs.Items[0]
}

// set the ResourceRequest status to deleted
rr.Status.OfferWithdrawalTimestamp = &now
obj, err = controller.crdClient.Resource("resourcerequests").Namespace(tenantNamespace.Name).UpdateStatus(rr.Name, rr, &metav1.UpdateOptions{})
Expect(err).To(BeNil())
Expect(obj).NotTo(BeNil())

rr, ok = obj.(*discoveryv1alpha1.ResourceRequest)
Expect(ok).To(BeTrue())
Expect(rr).NotTo(BeNil())

// call for the second time the unpeer function to delete the ResourceRequest
err = controller.unpeerNamespaced(ctx, fc)
Expect(err).To(BeNil())

// get the resource requests in the local tenant namespace
obj, err = controller.crdClient.Resource("resourcerequests").Namespace(tenantNamespace.Name).List(&metav1.ListOptions{})
Expect(err).To(BeNil())
Expect(obj).NotTo(BeNil())

rrs, ok = obj.(*discoveryv1alpha1.ResourceRequestList)
Expect(ok).To(BeTrue())
Expect(rrs).NotTo(BeNil())

Expect(len(rrs.Items)).To(BeNumerically("==", 0))
},

Entry("unpeer", unpeerTestcase{
Expand Down Expand Up @@ -512,7 +552,7 @@ var _ = Describe("ForeignClusterOperator", func() {
AuthURL: "",
},
},
expectedPeeringLength: Equal(0),
expectedPeeringLength: Equal(1),
expectedOutgoing: Equal(discoveryv1alpha1.Outgoing{
PeeringPhase: discoveryv1alpha1.PeeringPhaseDisconnecting,
}),
Expand Down
Expand Up @@ -64,7 +64,8 @@ func (r *ForeignClusterReconciler) createResourceRequest(ctx context.Context,
klog.Error(err)
return controllerutil.OperationResultNone, err
}
klog.Infof("[%v] ensured the existence of ResourceRequest (with %v operation)", remoteClusterID, result)
klog.Infof("[%v] ensured the existence of ResourceRequest (with %v operation)",
remoteClusterID, result)

return result, nil
}
Expand Down
28 changes: 22 additions & 6 deletions internal/resource-request-operator/resourceRequest_controller.go
Expand Up @@ -37,9 +37,9 @@ const (
// +kubebuilder:rbac:groups=capsule.clastix.io,resources=tenants,verbs=get;list;watch;create;update;patch;delete;

// Reconcile is the main function of the controller which reconciles ResourceRequest resources.
func (r *ResourceRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
func (r *ResourceRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) {
var resourceRequest discoveryv1alpha1.ResourceRequest
err := r.Get(ctx, req.NamespacedName, &resourceRequest)
err = r.Get(ctx, req.NamespacedName, &resourceRequest)
if err != nil {
klog.Errorf("unable to get resourceRequest %s: %s", req.NamespacedName, err)
return ctrl.Result{}, nil
Expand Down Expand Up @@ -80,10 +80,26 @@ func (r *ResourceRequestReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, err
}

err = r.generateResourceOffer(ctx, &resourceRequest)
if err != nil {
klog.Errorf("%s -> Error generating resourceOffer: %s", remoteClusterID, err)
return ctrl.Result{}, err
defer func() {
newErr := r.Client.Status().Update(ctx, &resourceRequest)
if newErr != nil {
klog.Error(newErr)
err = newErr
}
}()

if resourceRequest.Spec.WithdrawalTimestamp.IsZero() {
err = r.generateResourceOffer(ctx, &resourceRequest)
if err != nil {
klog.Errorf("%s -> Error generating resourceOffer: %s", remoteClusterID, err)
return ctrl.Result{}, err
}
} else {
err = r.invalidateResourceOffer(ctx, &resourceRequest)
if err != nil {
klog.Errorf("%s -> Error invalidating resourceOffer: %s", remoteClusterID, err)
return ctrl.Result{}, err
}
}

return ctrl.Result{}, nil
Expand Down

0 comments on commit 1c9f792

Please sign in to comment.