HTTP import with DIC - Design Proposal #243

ido106 · 2023-09-14T12:46:43Z

Signed-off-by: Ido Aharon [email protected]

Signed-off-by: Ido Aharon <[email protected]>

kubevirt-bot · 2023-09-14T12:46:53Z

Hi @ido106. Thanks for your PR.

I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

awels · 2023-09-20T12:27:37Z

design-proposals/data-import-cron-http-import.md

+## Motivation
+Currently, the DataImportCron allows the import of Registry Imports only. Recently, there was a demand to also allow HTTP import types.  
+The problem that arises from HTTP imports is that there is no convention between the different sources, so it's hard to know when the image is updated for each source in a generic way, which will make the polling process more difficult than standard registry sources.  
+One possible solution If the URL is static is to use a Get request with an If-Modified-Since header where we specify the date from which we want to check if there was a change. If there was a change since the specified date, the request will return with a status of 200OK, and then we know that the image has been updated since that date.  


So we can do the If-Modified-Since header on a schedule right? We know the time the job kicked off last so we pass that with the header. Do we know if all http servers support that header, I suspect not all of them do.

It may be that not all servers will support this header and then we will have to use another way.
Alex suggested that we can offload polling entirely to the user
@akalenyu

Yeah just something I thought about.. we could provide an example poller impl.
and require folks to supply a poller URL at the webhook level for HTTP sources (or "static" to bypass polling altogether)

Maybe limit the options of 'poller' type to a few known ones, including 'bypass' and 'if-modified-since'. We could expand that later if we identify more options. Maybe include the template idea from below, not sure.

I still like offloading the polling logic to the user. But second thoughts about running it on their behalf since it means a user app will run in the CDI namespace. We could always just disable our poller and have them create their own cronjobs

Maybe we can do it with the checksum like @awels suggested, but there is no unique convention for the file structure

awels · 2023-09-20T12:30:05Z

design-proposals/data-import-cron-http-import.md

+
+# Design
+
+If the URL is static:


I don't understand what you mean by If the URL is static by definition the URL passed to the Datavolume or DataImportCron is static, it cannot be modified. Is there some other property we are interested in? I don't get it.

Sometimes the image has the same URL and then we can use the way I mentioned, but sometimes the suffix changes when you upload a new version and we want to identify such a case, so we have to use another way.
When such case happens we want to update the AnnSourceDesiredDigest annotation to be the new suffix.

Honestly, I don't think we should consider that particular scenario. From my perspective that is a completely different file. The URL that is passed in is always static and cannot change. There would be no mechanism that allows us to reliably know if a different file is newer than the original. I guess we could consider a 'template' url in combination with some version scheme, but that would be an enhancement IMO.

awels · 2023-09-20T12:35:42Z

design-proposals/data-import-cron-http-import.md

+If the URL is static:
+* Make a GET request with the If-Modified-Since Header starting from the date stored in the AnnLastCronTime annotation
+* If the returned status is 200OK, perform import again
+* Update AnnLastCronTime to time.Now()


Have you considered that some sources provide a separate file with the hash of the image? For instance the fedora cloud images have a file that contains the checksum of the file. So we could download that file and verify the hash against what we have and decide if a new one exists that way.

Good point. The question is what percentage of sources support this and whether all sources provide the file in the same format?
For example https://cloud.debian.org/images/cloud/bookworm/daily/latest/SHA512SUMS looks different from the file you mentioned

As far as I can tell these are the areas where this file structure diverges:

Hash algorithm (MD5 in tinycore, SHA512 in debian, SHA256 in Fedora)

File structure (filename hash in most but fedora does SHA256 (filename) = hash)

Maybe we can catalog these, if there is a limited number of permutations of these formats, maybe we can implement them, and only support those limited number of options. I understand there might be many, but if we capture the most common ones, that might be enough.

awels · 2023-09-20T12:38:29Z

design-proposals/data-import-cron-http-import.md

+    spec:
+      source:
+        http:
+          url: "http://tinycorelinux.net/14.x/x86/release/Core-14.0.iso"


Just to bring home the point of the separate file, http://tinycorelinux.net/14.x/x86_64/release/ also contains a file with an md5 hash. So I think it might make sense to extend the http source with a hash_url that contains a hash of the actual source. Then we just need to write some code to parse that and compare it.

If the URL is static then it can help, if it changes so we need to find out the new suffix in addition to the hashcode to know the new URL. There could be other issues like the format of the checksum file.
This can help with Fedora case, for example Fedora-Cloud-Base-38-1.6.x86_64.qcow2 supposed to stay the same.

Maybe we can use the name of the version that is in the same file you mentioned (e.g CorePure64-14.0.iso). The question is whether the name always changes when the hashcode changes or it is possible to push changes to an existing image even if it is not named differently.

If for some sources the filename Fedora-Cloud-Base-38-1.6.x86_64 changes things get more
tricky for us, we might need some API on the DV level that is similar to sourceRef which derives
the new filename from the checksum file at creation time

Maybe some templating type thing could be useful. But I think we should start out with just considering 'static' URLs, get that working properly, then expand to more complex use cases if what we have is not sufficiently powerful.

kubevirt-bot · 2024-01-10T17:58:05Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

kubevirt-bot · 2024-02-09T18:21:40Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

aburdenthehand · 2024-02-21T12:12:06Z

@ido106 Do you still want to continue with this proposal? If so I'll remove the lifecycle labels.
From what I can tell it still has some unresolved questions raised on it.

kubevirt-bot · 2024-03-22T13:09:14Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

kubevirt-bot · 2024-03-22T13:09:17Z

@kubevirt-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ido106 · 2024-05-01T12:14:03Z

/reopen

kubevirt-bot · 2024-05-01T12:14:06Z

@ido106: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ido106 · 2024-05-01T12:17:58Z

@awels So as far as I understand, you suggest that at first we only handle instances that support the If-Modified-Since header, while assuming static links only ?

awels · 2024-05-01T12:25:17Z

@ido106 welcome back, glad you are doing well. Yes, lets start simple and then enhance if we find that what we did is not sufficient.

ido106 · 2024-05-01T13:38:24Z

@awels Thanks !
Okay, I will edit the design proposal soon

ido106 · 2024-05-01T15:56:31Z

@arnongilboa what do you think?

Signed-off-by: Ido Aharon <[email protected]>

kubevirt-bot · 2024-06-01T13:08:25Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

kubevirt-bot · 2024-06-01T13:08:28Z

@kubevirt-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

arnongilboa · 2024-06-02T14:35:35Z

/reopen

kubevirt-bot · 2024-06-02T14:35:40Z

@arnongilboa: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

arnongilboa · 2024-06-02T14:38:43Z

@arnongilboa what do you think?

Supporting HTTP header If-Modified-Since smells like a good starting point.

kubevirt-bot · 2024-07-02T14:54:55Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

kubevirt-bot · 2024-07-02T14:54:58Z

@kubevirt-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

aburdenthehand · 2024-07-02T14:58:47Z

/remove-lifecycle rotten

kubevirt-bot · 2024-09-30T15:42:53Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

kubevirt-bot · 2024-10-30T16:27:07Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

jean-edouard · 2024-11-04T13:43:17Z

@awels @akalenyu We should probably make a decision about this design proposal. Ignoring the details, do you agree with the general idea? Thanks

awels · 2024-11-04T13:56:54Z

Yes the general idea is sound, the problem is the details. In particular how do you detect changed images when the http server doesn't provide the needed details. But the feature definitely something we want.

kubevirt-bot · 2024-12-04T14:14:23Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

kubevirt-bot · 2024-12-04T14:14:27Z

@kubevirt-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

kubevirt-bot · 2025-01-15T17:01:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jean-edouard for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

http import with dic design proposal

9100913

Signed-off-by: Ido Aharon <[email protected]>

kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Sep 14, 2023

kubevirt-bot requested review from jean-edouard and jobbler September 14, 2023 12:46

kubevirt-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 14, 2023

kubevirt-bot added the size/M label Sep 14, 2023

awels reviewed Sep 20, 2023

View reviewed changes

kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 10, 2024

kubevirt-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 9, 2024

kubevirt-bot closed this Mar 22, 2024

kubevirt-bot reopened this May 1, 2024

new design proposal

99cffdc

Signed-off-by: Ido Aharon <[email protected]>

ido106 force-pushed the dic_http_design branch from 777da98 to 99cffdc Compare May 2, 2024 10:09

kubevirt-bot closed this Jun 1, 2024

kubevirt-bot reopened this Jun 2, 2024

kubevirt-bot closed this Jul 2, 2024

aburdenthehand reopened this Jul 2, 2024

kubevirt-bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jul 2, 2024

kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 30, 2024

kubevirt-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 30, 2024

kubevirt-bot closed this Dec 4, 2024

aburdenthehand reopened this Jan 15, 2025

HTTP import with DIC - Design Proposal #243

Are you sure you want to change the base?

HTTP import with DIC - Design Proposal #243

Conversation

ido106 commented Sep 14, 2023

kubevirt-bot commented Sep 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akalenyu Oct 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akalenyu Oct 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kubevirt-bot commented Jan 10, 2024

kubevirt-bot commented Feb 9, 2024

aburdenthehand commented Feb 21, 2024

kubevirt-bot commented Mar 22, 2024

kubevirt-bot commented Mar 22, 2024

ido106 commented May 1, 2024

kubevirt-bot commented May 1, 2024

ido106 commented May 1, 2024 • edited Loading

awels commented May 1, 2024

ido106 commented May 1, 2024

ido106 commented May 1, 2024

kubevirt-bot commented Jun 1, 2024

kubevirt-bot commented Jun 1, 2024

arnongilboa commented Jun 2, 2024

kubevirt-bot commented Jun 2, 2024

arnongilboa commented Jun 2, 2024

kubevirt-bot commented Jul 2, 2024

kubevirt-bot commented Jul 2, 2024

aburdenthehand commented Jul 2, 2024

kubevirt-bot commented Sep 30, 2024

kubevirt-bot commented Oct 30, 2024

jean-edouard commented Nov 4, 2024

awels commented Nov 4, 2024

kubevirt-bot commented Dec 4, 2024

kubevirt-bot commented Dec 4, 2024

kubevirt-bot commented Jan 15, 2025

akalenyu Oct 1, 2023 •

edited

Loading

akalenyu Oct 1, 2023 •

edited

Loading

ido106 commented May 1, 2024 •

edited

Loading