Commit Graph

15 Commits

Author SHA1 Message Date
akalenyu
50efcee3c2
[release-v1.57] Backport main commits to 1.57 release branch v2 (#2785)
* dataimportcron: Pass dynamic credential support label (#2760)

* dataimportcron: code change: Use better matchers in tests

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>

* dataimportcron: Pass dynamic credential support label

The label is passed from DataImportCron to DataVolume
and DataSource.

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>

---------

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>

* Add DataImportCron snapshot sources docs (#2747)

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* add akalenyu as approver, some others as reviewers (#2766)

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* Run `make rpm-deps` (#2741)

* Run make rpm-deps

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Avoid overlayfs error message by using vfs driver

Signed-off-by: Maya Rashish <mrashish@redhat.com>

---------

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Fix Destructive test lane failure - missing pod following recreate of CDI (#2744)

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* [WIP] Handle nil ptr in dataimportcron controller (#2769)

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Revert some gomega error checking that produce confusing output (#2772)

One of these tests flakes, but the error is hard to debug because
gomega will yell about
`Unexpected non-nil/non-zero argument at index 0`
Instead of showing the error.

Apparently this is intended:
https://github.com/onsi/gomega/pull/480/files#diff-e696deff1a5be83ad03053b772926cb325cede3b33222fa76c2bb1fcf2efd809R186-R190

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Run bazelisk run //robots/cmd/uploader:uploader -- -workspace /home/prow/go/src/github.com/kubevirt/project-infra/../containerized-data-importer/WORKSPACE -dry-run=false (#2770)

Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com>

* [CI] Add metrics name linter (#2774)

Signed-off-by: Aviv Litman <alitman@redhat.com>

---------

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>
Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
Signed-off-by: Maya Rashish <mrashish@redhat.com>
Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com>
Signed-off-by: Aviv Litman <alitman@redhat.com>
Co-authored-by: Andrej Krejcir <akrejcir@gmail.com>
Co-authored-by: Michael Henriksen <mhenriks@redhat.com>
Co-authored-by: Maya Rashish <mrashish@redhat.com>
Co-authored-by: kubevirt-bot <kubevirtbot@redhat.com>
Co-authored-by: Aviv Litman <64130977+avlitman@users.noreply.github.com>
2023-07-06 16:05:38 +02:00
kubevirt-bot
b80ff58f9f
[release-v1.57] DataVolume Controller uses VolumeCloneSource Populator (#2783)
* remove CSI clone

bye bye

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* no more smart clone

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* PVC clone same namespace

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* cross namespace pvc clone

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* various fixes to get some functional tests to work

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* delete smart clone controller again

somehow reappeared after rebase

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* mostly pvc clone functional test fixes

make sure size detect pod only runs on kubevirt content type

clone populator was skipping last round op applying pvc' annotations

various func test fixes

review comments

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* more various test fixes

host clone phase should (implicitly) wait for clone source pod to exit

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* remove "smart" clone from snapshot

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* DataVolume clone from snapshot uses populator

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* improve clone populator/datavolume coordination on "running" condition

For host clone, not much changes, values still comming from annotations on host clone PVC

For smart/csi clone the DataVolume will be "running" if not in pending or error phase

Will have the same values for terminal "completed" state regardless of clone type

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* unit tests for pvc/snapshot clone controllers

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* remove skipped test added in https://github.com/kubevirt/containerized-data-importer/pull/2759

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* attempt address AfterSuite and generate-verify failures

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* handle snapshot clone with no target size specified

also add more validation to some snapshot clone tests

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* remove Patch calls

Using the controller runtime Patch API with controller runtime cached client seems to be a pretty bad fit

At least given the way the CR API is designed where an old object is compared to new.

I like patch in theory though and will revisit

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* Clone populator should plan and execute even if PVC is bound

It was possible to miss "preallocation applied" annotation otherwise

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* add long term token to datavolume

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* Rename ProgressReporter to StatusReporter

Should have been done back when annotations were addded to "progress"

Also, if pvc is bound do not call phase Reconcile functions only Status

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

---------

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
Co-authored-by: Michael Henriksen <mhenriks@redhat.com>
2023-06-30 20:16:35 +02:00
Alexander Wels
4b2c171ecc
Backport main commits to 1.57 release branch (#2764)
* Enable empty schedule in DataImportCron (#2711)

Allow disabling DataImportCron schedule and support external trigger

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* expand upon #2721 (#2731)

Need to replace requeue bool with requeue duration

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* Add clone from snapshot functionalities to clone-populator (#2724)

* Add clone from snapshot functionalities to the clone populator

Signed-off-by: Alvaro Romero <alromero@redhat.com>

* Update clone populator unit tests to cover clone from snapshot capabilities

Signed-off-by: Alvaro Romero <alromero@redhat.com>

* Fix storage class assignation in temp-source claim for host-assisted clone from snapshot

This commit also includes other minor and styling-related fixes

Signed-off-by: Alvaro Romero <alromero@redhat.com>

---------

Signed-off-by: Alvaro Romero <alromero@redhat.com>

* Prepare CDI testing for the upcoming non-CSI lane (#2730)

* Update functional tests to skip incompatible default storage classes

Signed-off-by: Alvaro Romero <alromero@redhat.com>

* Enable the use of non-csi HPP in testing lanes

This commit modifies several scripts to allow the usage of classic HPP as the default SC in tests.

This allows us to test our non-populator flow with a non-csi provisioner.

Signed-off-by: Alvaro Romero <alromero@redhat.com>

---------

Signed-off-by: Alvaro Romero <alromero@redhat.com>

* Allow snapshots as format for DataImportCron created sources (#2700)

* StorageProfile API for declaring format of resulting cron disk images

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Integrate recommended format in dataimportcron controller

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Take snapclass existence into consideration when populating cloneStrategy and sourceFormat

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

---------

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Remove leader election test (#2745)

Now that we are using the standard k8s leases from
the controller runtime library, there is no need to
test our implementation as it is no longer in use.
This will save some testing time and random failures.

Signed-off-by: Alexander Wels <awels@redhat.com>

* Integration of Data volume using CDI populators (#2722)

* move cleanup out of dv deletion

It seemed off to call cleanup in the prepare function
just because we don't call cleanup unless the dv is deleting.
Instead we check in the clenup function itself if it should be
done: in this 2 specific cases in case of deletion and in case
the dv succeeded.
The cleanup will be used in future commit also for population cleanup
which we also want to happen not only on deletion.

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Use populator if csi storage class exists

Add new datavolume phase PendingPopulation to
indicate wffc when using populators, this new
phase will be used in kubevirt in order to know
that there is no need for dummy pod to pass wffc phase
and that the population will occur once creating the vm.

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Update population targetPVC with pvc prime annotations

The annotations will be used to update dv that uses the
populators.

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Adjust UT with new behavior

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* updates after review

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Fix import populator report progress

The import pod should be taken from pvcprime

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Prevent requeue upload dv when failing to find progress report pod

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Remove size inflation in populators

The populators are handling existing PVCs.
The PVC already has a defined requested size,
inflating the PVC' with fsoverhead will only be
on the PVC' spec and will not reflect on the target
PVC, this seems undesired.
Instead if the populators is using by PVC that the
datavolume controller created the inflation will happen
there if needed.

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Adjust functional tests to handle dvs using populators

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Fix clone test

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* add shouldUpdateProgress variable to know if need to update progress

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Change update of annotation from denied list to allowed list

Instead if checking if the annotation on pvcPrime is not desired
go over desired list and if the annotation exists add it.

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* fix removing annotations from pv when rebinding

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* More fixes and UT

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* a bit more updates and UTs

Signed-off-by: Shelly Kagan <skagan@redhat.com>

---------

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Run bazelisk run //robots/cmd/uploader:uploader -- -workspace /home/prow/go/src/github.com/kubevirt/project-infra/../containerized-data-importer/WORKSPACE -dry-run=false (#2751)

Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com>

* Allow dynamic linked build for non bazel build (#2753)

The current script always passes the static ldflag to the
compiler which will result in a static binary. We would like
to be able to build dynamic libraries instead.

cdi-containerimage-server has to be static because we
are copying it into the context of a container disk container
which is most likely based on a scratch container and has no
libraries for us to use.

Signed-off-by: Alexander Wels <awels@redhat.com>

* Disable DV GC by default (#2754)

* Disable DV GC by default

DataVolume garbage collection is a nice feature, but unfortunately it
violates fundamental principle of Kubernetes. CR should not be
auto-deleted when it completes its role (Job with TTLSecondsAfter-
Finished is an exception), and once CR was created we can assume it is
there until explicitly deleted. In addition, CR should keep idempotency,
so the same CR manifest can be applied multiple times, as long as it is
a valid update (e.g. DataVolume validation webhook does not allow
updating the spec).

When GC is enabled, some systems (e.g GitOps / ArgoCD) may require a
workaround (DV annotation deleteAfterCompletion = "false") to prevent
GC and function correctly.

On the next kubevirt-bot Bump kubevirtci PR (with bump-cdi), it will
fail on all kubevirtci lanes with tests referring DVs, as the tests
IsDataVolumeGC() looks at CDIConfig Spec.DataVolumeTTLSeconds and
assumes default is enabled. This should be fixed there.

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Fix test waiting for PVC deletion with UID

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Fix clone test assuming DV was GCed

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Fix DIC controller DV/PVC deletion when snapshot is ready

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

---------

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
Signed-off-by: Alvaro Romero <alromero@redhat.com>
Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
Signed-off-by: Alexander Wels <awels@redhat.com>
Signed-off-by: Shelly Kagan <skagan@redhat.com>
Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com>
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Michael Henriksen <mhenriks@redhat.com>
Co-authored-by: alromeros <alromero@redhat.com>
Co-authored-by: akalenyu <akalenyu@redhat.com>
Co-authored-by: Shelly Kagan <skagan@redhat.com>
Co-authored-by: kubevirt-bot <kubevirtbot@redhat.com>
Co-authored-by: Arnon Gilboa <agilboa@redhat.com>
2023-06-22 00:59:17 +02:00
Nahshon Unna Tsameret
4d14028d5b
enable ginkgolinter and fix findings (#2703)
Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
2023-05-04 13:07:36 +02:00
Nahshon Unna Tsameret
e6d2286dfb
golangci-lint: Enable errcheck (#2696)
* golangci-lint: enable errcheck and fix findings

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

* golangci=lint: exit if find something

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

---------

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
2023-04-25 20:40:16 +02:00
Arnon Gilboa
107f0b0c04
Remove embedded reconcile.Reconciler (#2698)
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
2023-04-24 20:37:00 +02:00
Michael Henriksen
8cb11f53d9
update k8s libs to 1.26. (#2687)
* update k8s libs to 1.26.

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* remove some checks in log messages, they're redundant, and the format has changed

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* use 1.26 lib function `CheckVolumeModeMismatches` and `CheckAccessModes`

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

---------

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
2023-04-18 19:30:40 +01:00
Nahshon Unna Tsameret
48166bdc8d
Enable additional linters to golangci-lint configuration (#2691)
* enable the typecheck linter in golangci-lint

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

* enable the staticcheck linter and fix findings

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

* golangci-lint: enable ineffassign and fix finding

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

---------

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
2023-04-17 13:30:28 +01:00
Nahshon Unna Tsameret
34a98256a2
Start integrate golangci lint into CI (#2675)
* Start adding the golangci-lint to CI

golangci-lint is a collection of many linters. This PR adds
golangci-lint to the CI. For strat, it enables the govet linter, and fix
its single finding.

The PR adds this linter to the `test-lint` Makefile target.

The new .golangci.yml file is the configuration for the linter.

golangci-lint version was set to the latest one - v1.52.2.
It is defined in hack/build/run-linters.sh

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

* golangci-lint: enable gosimple and fix findings

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

* golangci-lint: enable unused and fix findings

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

---------

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
2023-04-11 13:59:33 +01:00
Ido Aharon
184ee03acf
Pass DV labels to PVC (#2547)
* Pass DV labels to PVC

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Rebase and add utests

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Transferring annotations in smart clone and passing dv instead of client

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* CSI clone and Cloning from snapshot source functional tests

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* CSI clone and Cloning from snapshot source functional tests

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* uninitialized map case fixed

Signed-off-by: Ido Aharon <iaharon@redhat.com>

---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
2023-03-17 20:57:35 +00:00
Arnon Gilboa
12d57d6a94
Get rid of sync/updateStatus shared state (#2590)
There should be no state shared between sync() and updateStatus().
updateStatus() should stand on it's own, and come to it's own
conclusions based on what it observes. It is okay if it is "late to the
party" and does not observe the latest changes in sync(). It will
eventually converge. This is what kubevirt does.

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
2023-02-23 22:50:41 +01:00
Michael Henriksen
496efbcafb
Annotation to check for statically provisioned PVs when creating DataVolumes (#2583)
* function should return dataVolumeSyncResult, take *dataVolumeSyncResult as a parameter

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* checkStaticVolume implemetation for import DataVolume

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* upload support for checkStaticVolume

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* checkStaticVolume for clone datavolumes

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* checkStaticVolume for snapshot clone

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* checkStaticVolume for external populator source

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* tignten up static volume check

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* expand functional test to compare creation timestamps

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* updates from code review mostly add md5 verification to test and refacto common index creation

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* webhook changes, allow clone source DataVolumes (with special annotations)
even if source does not exist or user has no permission

BUT no token is added so this is really just for the static/prepopulate cases

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

---------

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
2023-02-22 23:40:48 +01:00
Arnon Gilboa
754961b2ce
Align DV controllers with kubevirt sync/updateStatus model (#2525)
* Align DV controllers with kubevirt sync/updateStatus model

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* CR fixes

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Update DV in sync if meta changed

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Fix meta and status update checks

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Fix DeepEqual check

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Some more CR fixes

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
2023-01-12 12:47:09 +00:00
Lee Yarwood
f229aeb5ff
dataimportcron: Pass KubeVirt instance type labels to DataVolume and DataSource (#2534)
* dataimportcron: Pass KubeVirt instance type labels to DataVolume and DataSource

Following on from 4fbcb2d509 a requirement
has arisen to expose the default instance type metadata previously
exposed as annotations also as labels to allow callers such as the UI to
have simple server side filtering of these resources.

The unreleased feature implementation in KubeVirt has now
switched to labels and so CDI should now do the same and pass through
the appropriate labels to the underlying resources.

Signed-off-by: Lee Yarwood <lyarwood@redhat.com>

* instancetype: Pass instance type labels from DataVolume to PVC

Unlike annotations not all labels are copied from a given DataVolume to
a PVC during an import. This change corrects this for instance type
labels ensuring they are passed down to the underlying PVC.

The associated constants are also moved into pkg/controller/common/util
to allow access from the DataImportCron and DataVolume controllers.

Signed-off-by: Lee Yarwood <lyarwood@redhat.com>

Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
2023-01-10 17:54:53 +00:00
Arnon Gilboa
bfe30a8964
Split and refactor DV controller (#2483)
- Split the huge DV controller into smaller op-specific DV controllers -
  import, clone, upload
- Add common watch-adding function so each controller watches only its
  relevant DVs
- Refactor the common Reconcile() to use interface DataVolumeReconciler
  implemented by each controller
- Move all functions, structs, consts to the relevant controller
- Split the utests per controller

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
2022-12-22 01:03:15 +00:00