* Test: Clone fs to block fails on size validation
When requesting size `X` with filesystem volume mode and storage api the size
is increased for the fs overhead. When trying to clone to block using
the same size `X` the clone fails because the target is smaller than source.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Improve size validation for clone
Skip size validation for filesystem in webhook and include filesystem
overhead when doing the validation in controller.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Correct size validation for smart clone
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Correct unit test with fs overhead
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Restore CDI Config after each clone test
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Review cleanup
Removing redundant conversions and not useful comments
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Modify datavolume admission webhook to enable creating clones without source PVC
This commit modifies the datavolume admission webhook to follow a more descriptive approach, enabling the creation of clones without a source PVC.
This clone will later be handled by the datavolume-controller until the source PVC is created.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Modify the datavolume-controller to improve the handling of clones without source
Since we are allowing the creation of clones without source PVC in the admission webhook, we need to improve the handling of these clones once in the datavolume-controller.
This commit modifies said controller, so we do proper error handling and validation until the source PVC is created.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Add unit tests to check the creation of clones without source PVC
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Include a mechanism to reconcile clones without source once the source PVC is created
This commit introduces a new datavolume-controller watch so, if a clone without source is created, we make sure to reconcile it once a proper PVC is created.
It also updates/includes unit tests for proper coverage.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Include functional tests to cover the creation of clones without source PVC
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Minor refactoring of clone-related code in DataVolume reconciler to improve readability
After enabling the creation of clones without source PVC in the datavolume controller, the clone-related logic outside its reconciler has increased in size and become sparse.
This commit rearranges all this code and condenses it into the clone reconciler, without changing the loop behavior.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Modify datavolume-mutate webhook to reject the creation of clones if the source PVC's namespace doesn't exist
In previous commits, a mechanism to allow the creation of clones without source PVC was added, without ever checking if the source PVC's namespace exists or not.
This behavior could lead to permission conflicts between the user and the source's namespace since the webhook skipped all the related validation.
This commit modifies the datavolume-mutate admission webhook to reject the clone if the source PVC's namespace doesn't exist.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Update unit tests for proper coverage of clone-validation functions
This commit adds and updates several unit tests to improve the coverage of the clone-validation mechanism after several functions were moved to the controller.
It also introduces minor changes on related code in the datavolume-controller and functional tests following PR review.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Allow empty DV size when cloning using storage API
When cloning a Data Volume, the size of the target can be potentially obtainable via the source PVC, which discards the need to explicitly specify it.
Considering that, this commit introduces a change in the correspondent validation webhook to allow omitting the resources.request.storage field when cloning a PVC using the storage API.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Modify datavolume-controller to allow obtaining storage size from source PVC when cloning
When cloning a PVC, if the target's size is not specified, said value can be attainable from the source PVC.
This commit introduces a change in datavolume controller so, in case of detecting an empty storage size, said value can be obtained when performing CSI and Smart cloning.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Update unit tests for datavolume-validation after enabling cloning with empty size
This commit updates the unit testing for the datavolume validation webhook, covering the possibility of cloning a PVC without setting any storage size.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Update unit testing for controller-related functions after enabling cloning with empty size
This commit includes unit tests for the volumeSize() function after enabling creating clones with blank size.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Update the datavolume controller to create a size-detection pod when performing host-assisted clone
When performing a host-assisted clone with empty clone size, simply copying the original PVC size could lead to potential overhead miscalculations if the source's VolumeMode is "filesystem".
When that's the case, an inspection pod will be created in the datavolume controller so it extracts the size of the virtual image using qemu-img.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Include an image-size detection tool to allow cloning with empty DV size
This commit introduces a new tool in charge of collecting the virtual image size when cloning with an empty DV size. In some cases where said value is unattainable from the original PVC's spec, the datavolume controller will create a new pod containing this new tool.
The binary will then run the 'qemu-img' command and handle its results appropriately.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Optimize the clone-size lookup process to avoid creating unnecessary size-detection pods
When performing host-assisted clone with an empty DV size, in some cases, a size-detection pod is used to obtain the required capacity.
This commit tries to optimize this process to keep the collected value as a PVC annotation, that is checked in subsequent clones to avoid creating redundant pods.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Minor fixes and improvements on mechanism for cloning with empty storage size
* Add new optional flag on size-detection binary to enable using a different URI scheme
* Improve the pod-creation mechanism so the pod is not created until the source PVC has finished the import
* Modify size-finlation mechanism to account for possible round-downs when importing the source image
* Improve the size inflation mechanism so only PVCs with filesystem as volume mode are considered
* Minor style corrections
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Modify the clone-controller to allow skipping the clone size validation in some cases
Due to filesystem overhead differences, the target's size can sometimes be smaller than the source's one when obtaining said value with the size-detection pod.
This commit introduces minor changes in the clone-controller so we can skip the size validation in those cases.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Minor changes and improvements in size-detection mechanism following PR review
* Added new UT that covers using empty storage API for non-cloning sources
* Added new watch on datavolume-controller that looks for changes in the size-detection pod
* Removed redundant and unnecessary specs on size-detection pod
* Added error handling when reading the pod's termination message
* Moved general-usage functions to 'util.go' file
* Updated 'datavolumes' documentation to reference the possibility of omitting the storage size when cloning
* Minor style corrections
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Add unit tests that cover the size-detection mechanism in the DataVolume controller
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Include functional tests for cloning without specifying storage size
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Improve error handling in the creation/deletion process of the size-detection pod
This commit introduces additional handling in case of error after and during the size-detection pod is created.
It also updates several related unit tests.
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Minor fixes to improve fsOverhead calculations when cloning with empty storage size
* Modified the size-detection mechanism so we account for fsOverhead when cloning to filesystem volume mode in all cases
* Clean up the code for reconciling when cloning a PVC that is not ready
* Minor fix in functional test so it works when cloning from block to filesystem volume mode
Signed-off-by: Alvaro Romero <alromero@redhat.com>
* Garbage Collect Completed DVs
See design at:
https://github.com/kubevirt/community/blob/main/design-proposals/garbage-collect-completed-dvs.md
ToDos:
-DataImportCron and DataSource controllers adaptation and func tests
-Add doc for DataVolume, CDIConfig and DataImportCron changes
-Extend unit tests and functional tests
-KubeVirt adaptation
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
* Controller minor fixes
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
* Adapt tests to GC
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
* Add DV mutate unit test for GC
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
* Improve GC skip per annotation test
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
* Use DescribeTable for the GC tests
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
* Detect storage capabilities for no-provisioner storage classes
Assume there's a persistent volume that we can look up to infer the
correct values for volume mode and access modes.
Limit ourselves to detecting no-provisioner capabilities on LSO to
avoid greatly increasing the number of storage classes we provide
capabilities for. This is similar to our current flow where we
only provide capabilities for known storage classes.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Regenerate bazel stuff for pkg/monitoring's existence
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add a watcher for no-provisioner PVs
We maintain a map of storage class names and provisioners whenever
storage classes are changed.
If a PV has one of the storage classes with no-provisioner as a
provisioner, reconcile that storage class.
This is because we infer the storage profile based on PVs, and
new ones might have different storage capabilities.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Use a client to do our storage class caching
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Pass a client as an argument, not global.
Suggested by awels, thanks!
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* periodic sync CSI snapshot CRD check
It was possible for the CSI snapshot CRD check to fail silently and
prevent the smart clone controller from starting during the cdi deployment
pod start up. This would prevent smart clone from working properly.
This adds a periodic sync of 1 minute for checking the CRDs. We also
log failures that are not is not found so we can more easily detect this
situation as humans.
Signed-off-by: Alexander Wels <awels@redhat.com>
* Change location of the start controller call.
Signed-off-by: Alexander Wels <awels@redhat.com>
* Introduce controller-runtime-sdk api package
Split controller-runtime-sdk into the base package and
controller-runtime-sdk/api.
Signed-off-by: Roman Mohr <rmohr@redhat.com>
* go mod vendor
Signed-off-by: Roman Mohr <rmohr@redhat.com>
* Update code references
Signed-off-by: Roman Mohr <rmohr@redhat.com>
* Create a test for an overhead bug
This image size and filesystem overhead combination was experimentally determined
to reproduce bz#2064936 in CI when using ceph/rbd with a Filesystem mode PV since
the filesystem capacity will be constrained by the PVC request size.
Below is the problem it tries to recreate:
When validating whether an image will fit into a PV we compare the
image's virtual size to the filesystem's reported available space to
guage whether it will fit. The current calculation reduces the apparent
available space by the configured filesystem overhead value but the
overhead is already (mostly) factored into the result of Statfs. This
causes the check to fail for PVCs that are just large enough to
accommodate an image plus overhead (ie. when using the DataVolume
Storage API with filesystem PVs with capacity constrained by the PVC
storage request size).
This was not caught in testing because HPP does not have capacity
constrained PVs and we are typically testing block volumes in the ceph
lanes. It can be triggered in our CI by allocating a Filesystem PV on
ceph-rbd storage because these volumes are capacity constrained and
subject to filesystem overhead.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Fix a target pvc validation bug
Corrects the validation logic for target volume.
Below description of the original problem:
When validating whether an image will fit into a PV we compare the
image's virtual size to the filesystem's reported available space to
guage whether it will fit. The current calculation reduces the apparent
available space by the configured filesystem overhead value but the
overhead is already (mostly) factored into the result of Statfs. This
causes the check to fail for PVCs that are just large enough to
accommodate an image plus overhead (ie. when using the DataVolume
Storage API with filesystem PVs with capacity constrained by the PVC
storage request size).
This was not caught in testing because HPP does not have capacity
constrained PVs and we are typically testing block volumes in the ceph
lanes. It can be triggered in our CI by allocating a Filesystem PV on
ceph-rbd storage because these volumes are capacity constrained and
subject to filesystem overhead.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Improve the warning message
Removed redundant and misleading part about pvc size and update the simplification
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Remove useless test
The test checks that the validation logic takes fs Overhead into account.
New validation logic does not check fs overhead. So test is no longer
relevant.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Set htpp(s)_proxy to lower case env variable
CURL used by nbdkit doesn't read upper case http(s)_proxy environment
variables, and thus was not using the proxy. Changed the variable to
be lower case.
Added a significant number of tests to test many more variations of
using a proxy. Also added https + auth endpoint to the file-host
container, so we can test https + auth with the proxy.
Added https endpoint to proxy, so we can test an https proxy.
Cleaned up some of the error handling in the import controller for
the proxy, in particular if a trustedCAProxy is defined.
Fixed some of the cluster wide proxy configuration so it works properly
inside an openshift cluster.
Signed-off-by: Alexander Wels <awels@redhat.com>
* Add https proxy support to registry import. Added extra
functional tests to test all registry import combinations
Signed-off-by: Alexander Wels <awels@redhat.com>
* Fixed some tests to work better in Open Shift.
Signed-off-by: Alexander Wels <awels@redhat.com>
* Update clone size validation logic
The case with DV using spec.storage API needs
more complex validation that will be added in the
clone controller. The API webhook validation
for that case is removed.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Improve DV phase failure message in tests
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Add test and warning event for clone size
During clone check if actual requested size on source volume is bigger
than target requested size and emit an event to notify user about situation.
Actual size on filesystem is lower that requested, because of possible filesystem overhead. When using storage API the overhead will be applied on target.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Code Review cleanup - Removing debug logs
Removed some garbage left after troubleshooting.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Move fn GetUsableSpace to common utils
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Add optional VDDK initImageURL field.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Pass VDDK image URL through to PVC annotation.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Unit tests for per-DV VDDK image URL.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Functional test for VDDK initImageURL field.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Update documentation for VDDK initImageURL.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Fix lint error.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Check for absence of AwaitingVDDK in unit test.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Update datavolume conditions when quota exceeded when creating pvc
When creating the pvc from the dv the pvc size
can exceed the allowed quota, in such case so far the only
indication was to look in the logs.
Now added indication in the data volume conditions
(when possible) and emitted event.
Signed-off-by: Shelly Kagan <skagan@redhat.com>
* Add functional tests to check the new conditons and event
Signed-off-by: Shelly Kagan <skagan@redhat.com>
* tests cosmetics
-use existing functions
-add missing checks on errors
-remove unused code
-etc..
Signed-off-by: Shelly Kagan <skagan@redhat.com>
* Update HTTP data source API to allow custom headers.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Implement custom HTTP headers API.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Document custom headers in HTTP data source.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Correct secretExtraHeader comment to reference Secret.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add volume mounts for secret headers.
Replaces environment variables for headers from secrets.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Avoid failing when there are no extra headers.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Redact contents of headers that come from secrets.
Also split up getExtraHeaders to reduce Sonar Cloud complexity.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Ensure all HTTP client requests use extra headers.
Missed redirect check and content length retrieval, both of which might
need the extra headers.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add some unit tests for extra HTTP headers.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Do not quote headers in nbdkit curl arguments.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add functional tests for extra HTTP headers.
Avoids new test server by specifiying basic authorization headers to the
existing file host port that requires it.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Use filepath.Walk to read secrets.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Minor documentation update for secrets.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Re-run 'make generate' for verification failure.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* move apis to new staging area
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* add script to push to staging
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* fix lint check and api reference
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* push staging to api repo
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* Add long term token (10 years) to pvcs when host assisted cloning between namespaces
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* clone controller should retry if source in use
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* minor refactor if/else -> switch
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
Users don't want 👽 resources in clusters,
and we should also be able to tell if were part of a broader installation.
Note:
- Operator created resources were handled in https://github.com/kubevirt/controller-lifecycle-operator-sdk/pull/18
as these labels will be common to all resources deployed by the HCO.
- Now that the controller is guaranteed to have the labels, we can set env vars
that reference the label values (fieldRef) to spare calling GET on the CR in the controllers.
(thanks mhenriks).
Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
* Strip newlines when writing a termination message.
Otherwise it isn't visible, at least when viewing in the -o yaml view.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Write down the nbdkit output and add it to the error output
With the added output from nbdkit, we can see the reason for the
non-existence of the nbdkit socket.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Don't set that we're waiting if it's CrashLoopBackOff
It's better to have the reason for the crash (terminate message)
than "backing off 5 minutes"
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Simplify all "image too large to fit" terminate reasons.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Move verifyConditions to utils, no functional change
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Switch test for image too large to test condition and not log
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Remove unused branch
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Rename setConditionFromPodWithPrefix to setAnnotationsFromPodWithPrefix
No functional change. Intended to be followed by some refactoring.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Fold restart count logic into the common setAnnotationsFromPodWithPrefix
Changing to >= rather than > to ensure a zero pod restart count is
always used -- the import controller unit tests request this.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Reduce indentation by returning right away.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Fold check for pod being nil into common code.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* move saveVddkAnnotations into util and make it unconditional
Call it setVddkAnnotations for consistency.
Check for not-terminated inside the function, not outside.
Removes check for source being VDDK (to avoid passing more arguments):
it won't match the regex anyway.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Reduce indentation by bailing on failure.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Reorder parameters to mirror the order in the function name
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Use a named variable for first container state
Yields shorter, more legible lines.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Use a constant for the nbdkit log.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add more information to function description - also logs to file
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add an interface to watch nbdkit logs.
Useful for fishing out various pieces of information. Save VDDK library
version and connected ESX host by appending to the importer pod's
termination message. Turns nbdkit logging up to verbose for VDDK data
sources, so only the last few lines are printed for debugging.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Copy VDDK info from termination message to PVC/DV.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add unit tests for saved VDDK information.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add functional test for VDDK annotations.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Fix unit test, forgot to check for nil pvc.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Don't ignore errors updating PVC with VDDK info.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Watch nbdkit with Scanner instead of ReadString.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Move VDDK info test into existing functional test.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Make nbdkit stop sequence slightly clearer.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Save VDDK info in regular DV reconciler.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Don't save VDDK info when PVC is being deleted.
Also, piggyback off existing PVC update instead of introducing a new
error handling path.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Fix VDDK-info unit tests.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Use scanner for all nbdkit logging.
Also fix up a minor merge mistake.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Try to satisfy complaints from SonarCloud.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* use namespace transfer for smart clone
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* updates from test failures
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* add expansion func tests
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* add dv phases for expansion and transfer
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* rebase and integrate with storage profiles
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* Create new Storage type
A new Storage type similar to the PVC Spec is now available to use
in the DataVolume Spec. This is more permissive than PVC, and together
with StorageProfile this allows CDI to apply additional logic for
missing or optional fields.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Use the StorageProfile
Handle the StorageProfile recommended params when creating the PVC for
a DataVolume. When parameters like volumeMode or accessModes are
not provided, CDI checks the StorageProfile for a given StorageClass
to set the recommended defaults. This enables user to create DataVolume
without the need to provide all the parameters.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Allow multiple accessModes
CDI allows multiple access modes to be specified in the DataVolume.spec.storage and in the StorageProfile. This now works the same way as in PVC specification.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Handle the storage.size field
The storage.size specifies how much space a user wants to have.
When creating image on the fileSystem storage CDI takes into
account the file system overhead and requests PVC big enough to
fit an image and file system metadata.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Test storage profile with DV
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Document Storage Profiles
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Refactor: Render the effective PVC early
The helper 'render PVC' was moved earlier in the control flow, so
it can be used in more places. Removing the need for if/else logic.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Test handling size on import, upload and clone
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Code Review: Refactor resolving of volumeMode
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Fix: render target pvc spec correctly in smart clone controller
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Refactoring - move PreallocationApplied flag definition to common
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* Preallocate cloning DataVolumes
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* Cluster-scoped namespace transfer api and controller
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* unit tests
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* ObjectTransfer webhook
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* new functests
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* experiment with termination grace period
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* quota test
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
when the annotation is not set explicitly in the DV/PVC
e.g. disable Istio sidecar injection by default
Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
* api: Add new ImportProxy structure to CDIConfig in v1beta1 core api
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* common: Add constants related to configure Import proxy in Import pod and controller reconcile
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* config controller: Add import proxy reconcile, monitoring OCP proxy updates
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* config controller: Add unit test for import proxy reconcile, monitoring OCP proxy updates and creating Secret and ConfigMap
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* import controller: Add unit support of ImportProxy in the tests
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* controller util: Add support to retrieve an OpenShift cluster wide proxy object as well as its needed configurations
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* controller util: Add unit test for the get cluster wide proxy functionality
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* controller: moved isOpenshift function to utils common and export it
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* importer: Clone http transport to keep the default proxy config such as the usage of environment variables and add support for proxy CA
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* operator: Update CDIConfigCRD with import proxy info
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* operator: Update CDIListCRD with import proxy info
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* operator: Add service account rights to CDI for accessing OpenShift proxy obj
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* operator: Add OpenShift proxy obj scheme support
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: Add new forwarding proxy for testing things that require a proxy
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: Add test to verify the importer pod with proxy
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* doc: Update cdi config doc with the ImportProxy info
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* add support to build the new proxy docker image
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: remove unwanted F parameter from test
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* controller: fix error handling
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: update the used method to get the pod of the importer pod and proxy
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* fixed comments from the revision
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* controller config: update the methods to use the log object from the reconcile object
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* controller: update function GetImportProxyConfig to return errors and include a new unit test
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* controller: add back a test that was removed by mistake
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* update updateCDIConfigByUpdatingTheClusterWideProxy function
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: decrease the import pod log pooling interval and increase the image size to be able to verify the import pod before it is deleted
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: update the pvc and proxied requests verification
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* tools: add X-Forwarded-For header in the proxy requests used for testing
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: reset cluster wide proxy with the original values
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* test: fix proxy update to change spec instead of status and other minor updates
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* doc: update import proxy description
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
* update generated files
Signed-off-by: Marcelo Amaral <marcelo.amaral1@ibm.com>
Printing just the name of the storageclass is just as descriptive
as printing the entire structure, and V(3) is probably preferred
for debug output.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add a DV/PVC annotation "storage.bind.immediate.requested"
When the annotation is applied the CDI will force bind the PVC (by scheduling worker pods), ignoring the logic to handle WaitForFirstConsumer binding mode.
This is useful when uploading "template" images to the cluster on local storage with WaitForFirstConsumer binding. In this case the image has to be available somewhere on the cluster, the actual placement of image on specific node is not important, so the CDI worker node can be used as a first consumer.
For storage with immediate binging it is effectively a NOOP.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Test import/upload/clone with annotation "storage.bind.immediate.requested"
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* [WIP] doc: User-facing doc for preallocation support
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* apis: CDI accepts `preallocation` option.
With this commit CDI accepts (but does handle) `preallocation` settings
for DataVolumes and in CDIConfig.
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* core: Implementing preallocation
This commit implements preallocation support for import and upload.
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* test: Functional tests for preallocation support
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* core: Remove "preallocation for StorageClasses" config
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* test: Removed unused function
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* test: Fix rook-ceph test failures
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* Updated dependencies
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* core: Uss PVC annotation to pass preallocation parameters
DataVolume controller now uses a PVC annotation to pass preallocation
configuration to import and update controllers.
Signed-off-by: Tomasz Baranski <tbaransk@redhat.com>
* Plumb new checkpoint API through to VDDK importer.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add incremental data copy from VDDK.
Create a new data source implementation similar to vddk-datasource, but
only for blocks of data that changed between two snapshots. Also factor
out common things between the two VDDK data sources.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Check block status for warm and cold imports.
Addresses a bunch of runtime issues, but progress tracking isn't right.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Find snapshots correctly.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Remove separate warm/cold VDDK importers.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Advance through the checkpoint list in the spec.
Move DataVolume to Paused after each checkpoint, and start a new
importer pod for the next available checkpoint. Keep track of which
checkpoints have been copied by adding PVC annotations associating each
checkpoint with the UID of the pod that copied it.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Allow spec updates to drive multi-stage imports.
A multi-stage import can create checkpoints at any time, so CDI needs to
be able to receive updates to the list of checkpoints. Implement this by
allowing spec changes only for fields related to multi-stage imports.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Avoid deleting destination in multi-stage import.
A multi-stage import will have an initial data copy to the destination
file followed by separate copies for individual deltas. The destination
file should not be deleted before starting these delta copies.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Get VDDK data source to pass formatting tests.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Unit tests for multi-stage import admission rules.
Make sure only updates to checkpoint-related fields are accepted.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add warm import unit tests for VDDK data source.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add VDDK warm import functional test.
Put two snapshots in the vCenter simulator inventory, and run them
through a multi-stage import process. Also clean up some issues
reported by test-lint.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add some documentation about multi-stage imports.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Pass existing multi-stage DataVolume unit tests.
Also remove MD5 sum step used for debugging, since it can take a long time.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Remove tabs from documentation.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Pass failing import-controller unit test.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* More unit tests for multi-stage field updates.
Also factor these tests into a DescribeTable.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add nbdkit retry filter.
Available as of Fedora 33 update.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Give correct file name to nbdkit in more cases.
The backing file in the spec might not always match the backing file in
the snapshot, so try harder to match those files by disk ID. May still
need to allow updates to backingFile, depending on how this gets used.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add more unit tests for datavolume-controller.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Fix linter error from last commit.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add unit tests for some govmomi API calls.
Move original calls into mock interfaces to make this work.
Signed-off-by: Matthew Arnold <marnold@redhat.com>
* Add checkpoints to DataVolume CRD and reconciliation
* Add Previous, Current, and FinalCheckpoint to DataVolume CRD
* Use checkpoints to set annotations on the PVC
* If an importer pod succeeds while checkpoint annotations are set,
then set the DataVolume status to Paused intstead of Succeeded.
* Clear the PVC checkpoint annotations
Signed-off-by: Sam Lucidi <slucidi@redhat.com>
* Add new fields to DataVolume CRD creation
Signed-off-by: Sam Lucidi <slucidi@redhat.com>
* Generate updated code for the DataVolume changes
Signed-off-by: Sam Lucidi <slucidi@redhat.com>
* Add tests for multistage import annotations
Signed-off-by: Sam Lucidi <slucidi@redhat.com>
* Add CDIConfigSpec to CDI
Make CDIConfig singleton mirror data in "active" CDI
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* fix functional tests
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
* check if pod used
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Test scratch deletion during import
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Handle scratch missing just after being created in utility function.
There was a possibility that just after creating the scratch it is deleted, and the next get results in error. This error was ignored - not returned from util - resulting in wrong behavior.
Other fix here is to skip pods with status PodSucceeded and PodFailed in getPodsUsingPVCs function - such Pods are not using PVC, as these pods are already done.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Reschedule reconcile for the pvc until pvc import is complete.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Increase number of NFS volumes, and document the test.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Try another quick fix for verifier pod flakiness.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* Cleanup defaults
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
* When validating disk space, reserve space for filesystem overhead
The amount of available space in a filesystem is not exactly
the advertise amount. Things like indirect blocks or metadata
may use up some of this space. Reserving it to avoid reaching
full capacity by default.
This value is configurable from the CDIConfig object spec,
both globally and per-storageclass.
The default value is 0.055, or "5.5% of the space is
reserved". This value was chosen because some filesystems
reserve 5% of the space as overhead for the root user and
this space doubles as reservation for the worst case
behaviour for unclear space usage. I've chosen a value
that is slightly higher.
This validation is only necessary because we use sparse
images instead of fallocated ones, which was done to have
reasonable alerts regarding space usage from various
storage providers.
---
Update CDIConfig filesystemOverhead status, validate, and
pass the final value to importer/upload pods.
Only the status values controlled by the config controller
are used, and it's filled out for all available storage
classes in the cluster.
Use this value in Validate calls to ensure that some of the
space is reserved for the filesystem overhead to guard from
accidents.
Caveats:
Doesn't use Default: to define the default of 0.055, instead
it is hard-coded in reconcile. It seems like we can't use a
default value.
Validates the per-storageClass values in reconcile, and
doesn't reject bad values.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Use util GetStorageClassByName
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Test filesystem overhead validation against async upload endpoint
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* wait for NFS PVs to be deleted before continuing
Intended to help with flakes, but didn't make a difference.
Probably still worth doing.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Avoid using the uncached client unnecessarily
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add error handling for the case where even a default SC is not found
Note that this change isn't expected to make a difference, as we
check if the targetStorageClass is nil later on and have the same
behaviour, but this is probably more correct API usage.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add testing for the validation of filesystem overhead values
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Fix logical error in waiting for NFS PVs.
Wait for all of them, not just the last one.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add library function to determine if a PVC has been populated fully.
The logic is as following:
If PVC has no ownerRef, then we assume something else fully populated it and
will return true
If PVC has an ownerRef and its a DataVolume, then look up the DataVolume
If DV.status.Phase == succeeded, return true, return false otherwise.
Signed-off-by: Alexander Wels <awels@redhat.com>
* Renamed functions to better indicate its purpose.
Signed-off-by: Alexander Wels <awels@redhat.com>
* Generate CDI CRD using controller-tools.
This is only done for CDI CRD as it requires the existence of source
code. Other CRDs we create are created by a more bare bones pod.
CDIUninstallStrategy was missing a comment describing it, so add
one. This was spotted manually so there might be more missing.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Allow users to specify which nodes CDI pods will live on.
nodeSelector, affinity and tolerations are possible values.
This is done in the CDI CR (rather than CDIConfig) as we are
interested in having this field be populated by external operators.
Unit tests now require the existence of a CDI CR, so create it.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Add a unit test covering some node placement functions
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Specify that all our pods are linux-only.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Avoid duplicate test, accidental left over.
Pointed out by awels, thanks.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Rename to cdiOperatorDeployment for clarity.
Suggested by awels
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Specify we only run on linux using the CDI CR, no need to embed this
into the code.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Don't dereference workloadPlacement for no reason
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Split off operator test to have its own AfterEach, BeforeEach.
Use even more descriptive function names.
Do all the CDI delete/restore logic in AfterEach, to ensure that
it happens and restores the deployment with the original CR even
if the test fails.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Remove XXX. This is the proper way.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Adapt to latest changes in controller_test.go (renaming import)
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Simplify, not storing intermediate value.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Don't dereference nodeplacement in callers to CreateDeployment
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Remove redundant save & restore. Unit tests do this for us.
Pointed out by awels, thanks.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Split out "find toplevel" to a utility function
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Wait for the CDI CR update to apply before continuing.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Simplify, not storing intermediate value.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
* Make it clear that the chosen node placement will not be schedulable.
Signed-off-by: Maya Rashish <mrashish@redhat.com>
