Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

During bootup, for existing applications with RW images, avoid waiting for verification of downloaded images. #481

Merged

Conversation

kalyan-nidumolu
Copy link
Contributor

During bootup, for existing applications with RW images, avoid waiting for verification of downloaded images.

  1. DomainMgr should publish AppUUID for the images in ImageStatus
  2. Zedmanager, when creating an App instance, should check if
    the image is already available in ImageStatus for RW images.
    if yes, it should just add Verifier config and skip waiting for
    verifier
  3. Verifier - Skip verifying images on bootup
    • These can be verified with the first verifier config is added.

Signed-off-by: Kalyan Nidumolu kalyan@zadeda.com

Fix Verifier to not wait for images in verified/ dir to be re-verified
after bootup.

Signed-off-by: Kalyan Nidumolu kalyan@zadeda.com

@kalyan-nidumolu
Copy link
Contributor Author

Am still testing the fix. Please review the fix.

@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch 3 times, most recently from 8117894 to e0b405d Compare January 2, 2020 11:15
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the design.
zedmanager doesn't need to wait for a verified image when a RW image already exists. That's OK

But you also seem to be removing the code in verifier which re-runs the verification and publication of the verified images on boot. I don't understand why (assuming that is what you are removing; didn't complete the review due to my lack of understanding of what you are trying to do)

@eriknordmark
Copy link
Contributor

3. Verifier - Skip verifying images on bootup

* These can be verified with the first verifier config is added.

Doesn't zedmanager look for a VerifyImageStatus (after it has looked for a RW image), and if none found it creates a DownloaderConfig? If so change #3 will result in a duplicate/unneeded download.

@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch 2 times, most recently from 12b8d61 to 4db3499 Compare January 3, 2020 19:05
@kalyan-nidumolu
Copy link
Contributor Author

kalyan-nidumolu commented Jan 3, 2020

I don't understand the design.
zedmanager doesn't need to wait for a verified image when a RW image already exists. That's OK

But you also seem to be removing the code in verifier which re-runs the verification and publication of the verified images on boot. I don't understand why (assuming that is what you are removing; didn't complete the review due to my lack of understanding of what you are trying to do)

Here is the design - basically what we discussed:

  1. Verifier - on bootup - published VerifiedImageStatus with state set to Download.
  2. When Zedmanager reacts to VerifiedImageStatus, it adds VerifierImageConfig
    -- Existing Code - No changes here
  3. Verifier reacts to VerifierImageConfig ( sees it as Modify ) - Does the Actual verification here..
  4. Zedmanager - When creating an app Instance - checks if the imageStatus already exists for the APP UUID - if yes - directly uses it. If not - Falls through the existing code path..

With these changes - verification of previously verified images happen in verifier.go -- handleModify --- And happens through the triggers explained above. Hence removed the current special code for verification..

This is the gist of the design..

@kalyan-nidumolu
Copy link
Contributor Author

  1. Verifier - Skip verifying images on bootup
* These can be verified with the first verifier config is added.

Doesn't zedmanager look for a VerifyImageStatus (after it has looked for a RW image), and if none found it creates a DownloaderConfig? If so change #3 will result in a duplicate/unneeded download.

No..Because currently, we check for VerifierImageStatus in zedmanager.. And verifier - during bootup - publishes VerifyImageStatus with State set too DOWNLOADED. this moved to DELIVERED ( practivally immediately as explained above ) .. But irrespective on when this move, the VerifyImageStatus ensures the image doesn't gett downloaded again..

@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch from 4db3499 to 7ba909d Compare January 3, 2020 19:23
@eriknordmark
Copy link
Contributor

  • Verifier reacts to VerifierImageConfig ( sees it as Modify ) - Does the Actual verification here..

That requires new code paths AFAIK.
When verifier handles a VerifyImageConfig it assumes the file is in /persist/downloaded/pending (and moves it via the verifying to the verified directories). If you want it to look in the verified directory and run the verification there you need that logic. Otherwise things are likely to break with failures about /persist/download/pending/ missing.

@kalyan-nidumolu
Copy link
Contributor Author

  • Verifier reacts to VerifierImageConfig ( sees it as Modify ) - Does the Actual verification here..

That requires new code paths AFAIK.
When verifier handles a VerifyImageConfig it assumes the file is in /persist/downloaded/pending (and moves it via the verifying to the verified directories). If you want it to look in the verified directory and run the verification there you need that logic. Otherwise things are likely to break with failures about /persist/download/pending/ missing.

I handled it in verifier.handleModify -- The code that was being executed during init - that just takes the image in the verified directory - and verifies it - I added similar code in handleModify. So, don't need to move it through pending/verifier/verified again..

@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch 2 times, most recently from 13759ce to 3cc7676 Compare January 6, 2020 10:32
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nits

pkg/pillar/types/verifiertypes.go Show resolved Hide resolved
pkg/pillar/cmd/zedmanager/handleimagestatus.go Outdated Show resolved Hide resolved
pkg/pillar/cmd/verifier/verifier.go Show resolved Hide resolved
@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch from 3cc7676 to 7a02de8 Compare January 6, 2020 19:19
@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch from 7a02de8 to 6ba6bce Compare January 7, 2020 11:15
@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch 3 times, most recently from a772a9f to a460a77 Compare January 7, 2020 19:23
for verification of downloaded images.

1) DomainMgr should publish AppUUID for the images in ImageStatus
2) Zedmanager, when creating an App instance, should check if
    the image is already available in ImageStatus for RW images.
    if yes, it should just add Verifier config and skip waiting for
    verifier
3) Verifier - Skip verifying images on bootup
    - These can be verified with the first verifier config is added.

Signed-off-by: Kalyan Nidumolu <kalyan@zadeda.com>

Fix Verifier to not wait for images in verified/ dir to be re-verified
after bootup.

Signed-off-by: Kalyan Nidumolu <kalyan@zadeda.com>
@kalyan-nidumolu kalyan-nidumolu force-pushed the kalyan-skipVerifyRwImage branch from a460a77 to ccab6cc Compare January 7, 2020 20:21
@eriknordmark eriknordmark merged commit c42f285 into lf-edge:master Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants