Skip to content

Commit

Permalink
Merge pull request #1862 from ncdc/0.7/skip-projected-resources
Browse files Browse the repository at this point in the history
[release-0.7] deletors: skip projected resources
  • Loading branch information
openshift-merge-robot authored Aug 31, 2022
2 parents bfed9f6 + be47352 commit 95799cf
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 7 deletions.
15 changes: 8 additions & 7 deletions pkg/informer/informer.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"

"github.com/kcp-dev/kcp/pkg/apis/tenancy"
metadataclient "github.com/kcp-dev/kcp/pkg/metadata"
"github.com/kcp-dev/kcp/pkg/projection"
)

const (
Expand Down Expand Up @@ -426,16 +426,17 @@ func (d *DynamicDiscoverySharedInformerFactory) updateInformers() {
group := parts[0]
version := parts[1]
resource := parts[2]
gvr := gvrFor(group, version, resource)

// Don't start a dynamic informer for tenancy.kcp.dev/v1beta1 Workspaces. These are a virtual projection of
// data from tenancy.kcp.dev/v1alpha1 ClusterWorkspaces. Starting an informer for them causes problems when
// the virtual-workspaces server is deployed separately. See https://github.com/kcp-dev/kcp/issues/1654 for
// more details.
if group == tenancy.GroupName && version == "v1beta1" && resource == "workspaces" {
// Don't start a dynamic informer for projected resources such as tenancy.kcp.dev/v1beta1 Workspaces
// (these are a virtual projection of data from tenancy.kcp.dev/v1alpha1 ClusterWorkspaces).
// Starting an informer for them causes problems when the virtual-workspaces server is deployed
// separately. See https://github.com/kcp-dev/kcp/issues/1654 for more details.
if projection.Includes(gvr) {
continue
}

latest[gvrFor(group, version, resource)] = struct{}{}
latest[gvr] = struct{}{}
}

// Grab a read lock to compare against d.informers to see if we need to start or stop any informers
Expand Down
39 changes: 39 additions & 0 deletions pkg/projection/projected_apis.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
Copyright 2022 The KCP Authors.
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
http://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 projection

import (
"k8s.io/apimachinery/pkg/runtime/schema"

tenancyv1beta1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1beta1"
)

var projectedAPIs map[schema.GroupVersionResource]struct{}

func init() {
projectedAPIs = map[schema.GroupVersionResource]struct{}{
tenancyv1beta1.SchemeGroupVersion.WithResource("workspaces"): {},
}
}

// Includes returns true if gvr is for a projected API. An API is projected if it is not stored in etcd and instead
// comes from some other data that is actually stored in etcd. For example, Workspaces (tenancy.kcp.dev/v1beta1) are
// projected; the real data comes from ClusterWorkspaces (tenancy.kcp.dev/v1alpha1).
func Includes(gvr schema.GroupVersionResource) bool {
_, exists := projectedAPIs[gvr]
return exists
}
8 changes: 8 additions & 0 deletions pkg/reconciler/apis/apibindingdeletion/apibinding_deletor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"k8s.io/klog/v2"

apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
"github.com/kcp-dev/kcp/pkg/projection"
)

type gvrDeletionMetadata struct {
Expand Down Expand Up @@ -58,6 +59,13 @@ func (c *Controller) deleteAllCRs(ctx context.Context, apibinding *apisv1alpha1.
Version: version,
}

// Don't try to delete projected resources such as tenancy.kcp.dev v1beta1 Workspaces - these are virtual
// projections and we shouldn't try to delete them. The projections will disappear when the real underlying
// data (e.g. ClusterWorkspaces) are deleted.
if projection.Includes(gvr) {
continue
}

deletionMetadata, err := c.deleteAllCR(ctx, logicalcluster.From(apibinding), gvr)
if err != nil {
deleteContentErrs = append(deleteContentErrs, err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1"
conditionsv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/third_party/conditions/apis/conditions/v1alpha1"
"github.com/kcp-dev/kcp/pkg/apis/third_party/conditions/util/conditions"
"github.com/kcp-dev/kcp/pkg/projection"
)

const (
Expand Down Expand Up @@ -395,6 +396,13 @@ func (d *workspacedResourcesDeleter) deleteAllContent(ctx context.Context, ws *t
}
deleteContentErrs := []error{}
for gvr, verbs := range groupVersionResources {
// Don't try to delete projected resources such as tenancy.kcp.dev v1beta1 Workspaces - these are virtual
// projections and we shouldn't try to delete them. The projections will disappear when the real underlying
// data (e.g. ClusterWorkspaces) are deleted.
if projection.Includes(gvr) {
continue
}

gvrDeletionMetadata, err := d.deleteAllContentForGroupVersionResource(ctx, wsClusterName, gvr, verbs, workspaceDeletedAt)
if err != nil {
// If there is an error, hold on to it but proceed with all the remaining
Expand Down

0 comments on commit 95799cf

Please sign in to comment.