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.

Sign up for GitHub

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

Jump to bottom

DataVolume Controller uses VolumeCloneSource Populator #2750

Merged
merged 19 commits into from
Jun 29, 2023
Merged

DataVolume Controller uses VolumeCloneSource Populator #2750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2285ceb
remove CSI clone
mhenriks May 31, 2023
1cd82ef
no more smart clone
mhenriks Jun 1, 2023
f7bc5ef
PVC clone same namespace
mhenriks Jun 7, 2023
c5260ae
cross namespace pvc clone
mhenriks Jun 10, 2023
fbce7ec
various fixes to get some functional tests to work
mhenriks Jun 13, 2023
28f9411
delete smart clone controller again
mhenriks Jun 13, 2023
5382013
mostly pvc clone functional test fixes
mhenriks Jun 14, 2023
e77f74c
more various test fixes
mhenriks Jun 15, 2023
7e677b2
remove "smart" clone from snapshot
mhenriks Jun 16, 2023
525af57
DataVolume clone from snapshot uses populator
mhenriks Jun 16, 2023
fd1beed
improve clone populator/datavolume coordination on "running" condition
mhenriks Jun 20, 2023
e6b618f
unit tests for pvc/snapshot clone controllers
mhenriks Jun 22, 2023
6e49650
remove skipped test added in https://github.com/kubevirt/containerize…
mhenriks Jun 22, 2023
7ca275c
attempt address AfterSuite and generate-verify failures
mhenriks Jun 22, 2023
c76d2d6
handle snapshot clone with no target size specified
mhenriks Jun 23, 2023
075b7f2
remove Patch calls
mhenriks Jun 26, 2023
691028e
Clone populator should plan and execute even if PVC is bound
mhenriks Jun 26, 2023
f605718
add long term token to datavolume
mhenriks Jun 27, 2023
7edeaee
Rename ProgressReporter to StatusReporter
mhenriks Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/cdi-controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func start() {
klog.Errorf("Unable to setup upload populator: %v", err)
os.Exit(1)
}
if _, err := populators.NewClonePopulator(ctx, mgr, log, clonerImage, pullPolicy, installerLabels); err != nil {
if _, err := populators.NewClonePopulator(ctx, mgr, log, clonerImage, pullPolicy, installerLabels, getTokenPublicKey()); err != nil {
klog.Errorf("Unable to setup clone populator: %v", err)
os.Exit(1)
}
Expand Down
1 change: 0 additions & 1 deletion pkg/controller/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ go_library(
"//pkg/monitoring:go_default_library",
"//pkg/operator:go_default_library",
"//pkg/storagecapabilities:go_default_library",
"//pkg/token:go_default_library",
"//pkg/util:go_default_library",
"//pkg/util/cert:go_default_library",
"//pkg/util/cert/fetcher:go_default_library",
Expand Down
24 changes: 8 additions & 16 deletions pkg/controller/clone-controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
"kubevirt.io/containerized-data-importer/pkg/common"
cc "kubevirt.io/containerized-data-importer/pkg/controller/common"
"kubevirt.io/containerized-data-importer/pkg/token"
"kubevirt.io/containerized-data-importer/pkg/util"
"kubevirt.io/containerized-data-importer/pkg/util/cert/fetcher"
"kubevirt.io/containerized-data-importer/pkg/util/cert/generator"
Expand Down Expand Up @@ -67,8 +66,7 @@ type CloneReconciler struct {
clientCertGenerator generator.CertGenerator
serverCAFetcher fetcher.CertBundleFetcher
log logr.Logger
longTokenValidator token.Validator
shortTokenValidator token.Validator
multiTokenValidator *cc.MultiTokenValidator
image string
verbose string
pullPolicy string
Expand All @@ -88,8 +86,7 @@ func NewCloneController(mgr manager.Manager,
client: mgr.GetClient(),
scheme: mgr.GetScheme(),
log: log.WithName("clone-controller"),
shortTokenValidator: cc.NewCloneTokenValidator(common.CloneTokenIssuer, apiServerKey),
longTokenValidator: cc.NewCloneTokenValidator(common.ExtendedCloneTokenIssuer, apiServerKey),
multiTokenValidator: cc.NewMultiTokenValidator(apiServerKey),
image: image,
verbose: verbose,
pullPolicy: pullPolicy,
Expand Down Expand Up @@ -249,10 +246,14 @@ func (r *CloneReconciler) reconcileSourcePod(ctx context.Context, sourcePod *cor
}

if len(pods) > 0 {
es, err := cc.GetAnnotatedEventSource(ctx, r.client, targetPvc)
if err != nil {
return 0, err
}
for _, pod := range pods {
r.log.V(1).Info("can't create clone source pod, pvc in use by other pod",
"namespace", sourcePvc.Namespace, "name", sourcePvc.Name, "pod", pod.Name)
r.recorder.Eventf(targetPvc, corev1.EventTypeWarning, cc.CloneSourceInUse,
r.recorder.Eventf(es, corev1.EventTypeWarning, cc.CloneSourceInUse,
"pod %s/%s using PersistentVolumeClaim %s", pod.Namespace, pod.Name, sourcePvc.Name)
}
return 2 * time.Second, nil
Expand Down Expand Up @@ -406,16 +407,7 @@ func (r *CloneReconciler) findCloneSourcePod(pvc *corev1.PersistentVolumeClaim)
}

func (r *CloneReconciler) validateSourceAndTarget(ctx context.Context, sourcePvc, targetPvc *corev1.PersistentVolumeClaim) error {
// first check for extended token
v := r.longTokenValidator
tok, ok := targetPvc.Annotations[cc.AnnExtendedCloneToken]
if !ok {
// if token doesn't exist, no prob for same namespace
tok = targetPvc.Annotations[cc.AnnCloneToken]
v = r.shortTokenValidator
}

if err := cc.ValidateCloneTokenPVC(tok, v, sourcePvc, targetPvc); err != nil {
if err := r.multiTokenValidator.ValidatePVC(sourcePvc, targetPvc); err != nil {
return err
}
contentType, err := ValidateCanCloneSourceAndTargetContentType(sourcePvc, targetPvc)
Expand Down
86 changes: 44 additions & 42 deletions pkg/controller/clone-controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
AnnUploadClientName: "uploadclient"}, nil)
reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil))
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
By("Verifying no source pod exists")
sourcePod, err := reconciler.findCloneSourcePod(testPvc)
Expect(sourcePod).To(BeNil())
Expand Down Expand Up @@ -147,11 +147,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
sourcePvc := cc.CreatePvc("source", "default", map[string]string{}, nil)
reconciler = createCloneReconciler(testPvc, sourcePvc, podFunc(sourcePvc))
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
By("Verifying no source pod exists")
sourcePod, err := reconciler.findCloneSourcePod(testPvc)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -197,11 +197,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
}
reconciler = createCloneReconciler(objs...)
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
By("Verifying no source pod exists")
sourcePod, err := reconciler.findCloneSourcePod(testPvc)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -269,11 +269,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
sourcePod.Namespace = "default"
reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil), sourcePod)
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "testPvc1", Namespace: "default"}})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("missing required " + AnnUploadClientName + " annotation"))
Expand All @@ -286,11 +286,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
sourcePod.Namespace = "default"
reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil), sourcePod)
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "testPvc1", Namespace: "default"}})
Expect(err).ToNot(HaveOccurred())
secret := &corev1.Secret{}
Expand All @@ -303,11 +303,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
cc.AnnCloneRequest: "default/source", cc.AnnPodReady: "true", cc.AnnCloneToken: "foobaz", AnnUploadClientName: "uploadclient", cc.AnnCloneSourcePod: "default-testPvc1-source-pod"}, nil)
reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil))
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
By("Verifying no source pod exists")
sourcePod, err := reconciler.findCloneSourcePod(testPvc)
Expect(err).ToNot(HaveOccurred())
Expand All @@ -324,11 +324,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
testPvc := createTargetPvcFunc()
reconciler = createCloneReconciler(testPvc, createSourcePvcFunc())
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
By("Verifying no source pod exists")
sourcePod, err := reconciler.findCloneSourcePod(testPvc)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -431,11 +431,11 @@ var _ = Describe("Clone controller reconcile loop", func() {
testPvc := createTargetPvcFunc()
reconciler = createCloneReconciler(testPvc, createSourcePvcFunc())
By("Setting up the match token")
reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default"
reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1"
By("Verifying no source pod exists")
sourcePod, err := reconciler.findCloneSourcePod(testPvc)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -721,8 +721,10 @@ func createCloneReconciler(objects ...runtime.Object) *CloneReconciler {
scheme: s,
log: cloneLog,
recorder: rec,
shortTokenValidator: &cc.FakeValidator{
Params: make(map[string]string, 0),
multiTokenValidator: &cc.MultiTokenValidator{
ShortTokenValidator: &cc.FakeValidator{
Params: make(map[string]string, 0),
},
},
image: testImage,
clientCertGenerator: &fakeCertGenerator{},
Expand Down
36 changes: 32 additions & 4 deletions pkg/controller/clone/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"sort"

"github.com/go-logr/logr"
snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
Expand All @@ -21,6 +22,17 @@ import (
"kubevirt.io/containerized-data-importer/pkg/util"
)

const (
// PendingPhaseName is the phase when the clone is pending
PendingPhaseName = "Pending"

// SucceededPhaseName is the phase when the clone is succeeded
SucceededPhaseName = "Succeeded"

// ErrorPhaseName is the phase when the clone is in error
ErrorPhaseName = "Error"
)

// IsDataSourcePVC checks for PersistentVolumeClaim source kind
func IsDataSourcePVC(kind string) bool {
return kind == "PersistentVolumeClaim"
Expand Down Expand Up @@ -55,10 +67,20 @@ func AddOwnershipLabel(label string, obj, owner metav1.Object) {
obj.GetLabels()[label] = string(owner.GetUID())
}

// IsSourceClaimReadyArgs are arguments for IsSourceClaimReady
type IsSourceClaimReadyArgs struct {
Target client.Object
SourceNamespace string
SourceName string
Client client.Client
Log logr.Logger
Recorder record.EventRecorder
}

// IsSourceClaimReady checks that PVC exists, is bound, and is not being used
func IsSourceClaimReady(ctx context.Context, c client.Client, namespace, name string) (bool, error) {
func IsSourceClaimReady(ctx context.Context, args *IsSourceClaimReadyArgs) (bool, error) {
claim := &corev1.PersistentVolumeClaim{}
exists, err := getResource(ctx, c, namespace, name, claim)
exists, err := getResource(ctx, args.Client, args.SourceNamespace, args.SourceName, claim)
if err != nil {
return false, err
}
Expand All @@ -71,16 +93,22 @@ func IsSourceClaimReady(ctx context.Context, c client.Client, namespace, name st
return false, nil
}

pods, err := cc.GetPodsUsingPVCs(ctx, c, namespace, sets.New(name), true)
pods, err := cc.GetPodsUsingPVCs(ctx, args.Client, args.SourceNamespace, sets.New(args.SourceName), true)
if err != nil {
return false, err
}

for _, pod := range pods {
args.Log.V(1).Info("Source PVC is being used by pod", "namespace", args.SourceNamespace, "name", args.SourceName, "pod", pod.Name)
args.Recorder.Eventf(args.Target, corev1.EventTypeWarning, cc.CloneSourceInUse,
"pod %s/%s using PersistentVolumeClaim %s", pod.Namespace, pod.Name, args.SourceName)
}

if len(pods) > 0 {
return false, nil
}

return cdiv1.IsPopulated(claim, dataVolumeGetter(ctx, c))
return cdiv1.IsPopulated(claim, dataVolumeGetter(ctx, args.Client))
}

// GetGlobalCloneStrategyOverride returns the global clone strategy override
Expand Down
16 changes: 14 additions & 2 deletions pkg/controller/clone/csi-clone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

// CSIClonePhaseName is the name of the csi clone phase
const CSIClonePhaseName = "CSIClone"

// CSIClonePhase is responsible for csi cloning a pvc
type CSIClonePhase struct {
Owner client.Object
Expand All @@ -31,7 +34,7 @@ var _ Phase = &CSIClonePhase{}

// Name returns the name of the phase
func (p *CSIClonePhase) Name() string {
return "CSIClone"
return CSIClonePhaseName
}

// Reconcile ensures a csi cloned pvc is created correctly
Expand All @@ -43,7 +46,16 @@ func (p *CSIClonePhase) Reconcile(ctx context.Context) (*reconcile.Result, error
}

if !exists {
ready, err := IsSourceClaimReady(ctx, p.Client, p.Namespace, p.SourceName)
args := &IsSourceClaimReadyArgs{
Target: p.Owner,
SourceNamespace: p.Namespace,
SourceName: p.SourceName,
Client: p.Client,
Log: p.Log,
Recorder: p.Recorder,
}

ready, err := IsSourceClaimReady(ctx, args)
if err != nil {
return nil, err
}
Expand Down