Commit Graph

7 Commits

Author SHA1 Message Date
alromeros
4c911fed27
Fix PVC deletion logic in populators (#3362)
* Fix PVC' deletion logic in CDI populators

This commit ensures that we only proceed with PVC' deletion after confirming that the target PVC is rebound.

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

* Refactor IsBound so we check PVC status instead of spec

PVC status should be the definitive source to know whether the PVC is bound or not. This commit updates our IsBound function to use status instead of spec.

It also updates unit tests and related code to match the new behavior.

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

* Add check for lost target PVC in populators

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

---------

Signed-off-by: Alvaro Romero <alromero@redhat.com>
2024-08-22 22:22:35 +02:00
Edu Gómez Escandell
0e750262a3
Enable autoformatting linters (#3179)
* Enable gofmt linter

From the docs:

> Gofmt checks whether code was gofmt-ed. By default this tool runs with
> -s option to check for code simplification.

https://golangci-lint.run/usage/linters/#gofmt

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

* Run gomft on the project

Ran this command after adding the gofmt linter:

	golangci-lint run ./... --fix

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

* Enable whitespace linter

From the docs:
> Whitespace is a linter that checks for unnecessary newlines at the
> start and end of functions, if, for, etc.

https://golangci-lint.run/usage/linters/#whitespace

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

* Run whitespace on the project

Ran this command after adding the whitespace linter:

	golangci-lint run ./... --fix

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

* Enable GCI linter

Per the docs:

> Gci controls Go package import order and makes it always deterministic.

https://golangci-lint.run/usage/linters/#gci

NOTE: I noticed that many files separate their imports in a particular
way, so I set the linter to enforce this standard.

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

* Run GCI on the project

Ran this command after adding the GCI linter:

	golangci-lint run ./... --fix

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

---------

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>
2024-04-24 13:52:22 +02:00
Ben Coxford
ebb9d31ac9
fix: propagating mesh annotations to transient pods (#3186)
* fix propagating mesh annotations to transient pods

Signed-off-by: bcoxford <ben.coxford@ncr.com>

* set pvc annotations on prep claim and size detection transient pods

Signed-off-by: bcoxford <ben.coxford@ncr.com>

* remove old annotations

Signed-off-by: bcoxford <ben.coxford@ncr.com>

* switch to target dv on pvc clone controller

Signed-off-by: bcoxford <ben.coxford@ncr.com>

* use metav1 object

Signed-off-by: bcoxford <ben.coxford@ncr.com>

---------

Signed-off-by: bcoxford <ben.coxford@ncr.com>
2024-04-16 22:32:47 +02:00
Edu Gómez Escandell
3c06e26bac
Enable unconvert linter (#3171)
* Enable unconvert linter

This linter's doc describes it as:

   The unconvert program analyzes Go packages to identify unnecessary
   type conversions; i.e., expressions T(x) where x already has type T.

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

* Unrestrict the number of linter warnings

It is best to show all warnings at once than to reveal them piece-meal,
particularly in CI where the feedback loop can be a bit slow.

By default, linters may only print the same message three times
(https://golangci-lint.run/usage/configuration/#issues-configuration)
The unconvert linter always prints the same message, so it specially
affected by this setting.

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

* Remove redundant type conversions

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>

---------

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>
2024-04-15 20:32:08 +02:00
alromeros
544c57af1b
Improve handling of bound temp PVC in clone populator (#3126)
There is a short window of time where a bound PVC can have a unpopulated "capacity" field in its status.

The clone-populator handles this case by returning an error. This approach is overkill as the issue is usually fixed instantly.

This commit improves the handling of this case and adds a unit tests.

Signed-off-by: Alvaro Romero <alromero@redhat.com>
2024-03-20 21:01:15 +01:00
Michael Henriksen
4ce9272c2c
DataVolume Controller uses VolumeCloneSource Populator (#2750)
* 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>
2023-06-30 00:14:54 +02:00
Michael Henriksen
f88fab69dc
PVC Clone Populator (#2709)
* touch up zero restoresize snapshot

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

* clone populator

only supports PVC source now

snapshot coming soon

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

* more unit tests

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

* unit test for clone populator

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

* func tests for clone populator

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

* move clone populator cleanup function to planner

other review comments

verifier pod should bount readonly

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

* add readonly flag to test executor pods

synchronize get hash calls

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

* increase linter timeout

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

* better/explicit readonly support for test pods

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

* check pv for driver info before looking up storageclass as it may not exist

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

* addressed review comments

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

* chooseStrategy shoud generate more events

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

---------

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
2023-05-24 05:11:52 +02:00