Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Adding -DD parameter to unzip command. Forces file replacement irrespective of timestamp. #345

Closed

Conversation

nharper285
Copy link
Contributor

Summary of the Pull Request

What is this about?
When running automated deployments, Andrew and I found that 'tools' were not being properly replaced with the updated versions if the deployment was created prior to the original instance deployment.
To explain the scenario, I've provided our own experience as an example.

Let's look at the following scenario:

  • MSR released version 1.9.0 at 2:55pm on 11/20/2020.
  • A 1.7.0 instance of OneFuzz was manually deployed by a user at 2:55pm on 11/23/2020.
  • The user then creates an Azure pipeline to automatically update the instance and triggers an update at 2:55pm on 11/24/2020

What ends up happening is that when the automated deployment occurs, it unzips the 1.9.0 deployment zip and compares the timestamps within that zip (2:55pm on 11/20/2020) to the timestamps currently in the 'tools' container within the function app (2:55pm on 11/23/2020). Because the container timestamps are more recent than the zip timestamps, the files are not replaced. We are then stuck in a situation where some of the tools, like the onefuzz-agent, are version 1.7.0 while the rest of the api have been properly updated to 1.9.0.

To mitigate this issue, we've added the -DD parameter to the unzip command.
The duplicated option -DD forces suppression of timestamp restoration for all extracted entries (files and directories). This option results in setting the timestamps for all extracted entries to the current time.

Info on Pull Request

What does this include?
Changes to the .yml deployment file. Specifically, we've added the -DD param to the unzip command.

Validation Steps Performed

We've tested this change in our deployment pipeline.

How does someone test & validate?
One can test by following a similar pattern of events as the ones detailed above. Without the param, the files are not updated. With the param, the files are properly updated.

@nharper285 nharper285 closed this Nov 25, 2020
@nharper285 nharper285 reopened this Nov 25, 2020
@@ -69,7 +69,7 @@ stages:
workingDirectory: "$(ONEFUZZ_DEPLOY_LOC)/$(artifact)"
script: |
set -ex
unzip onefuzz-deployment-$(onefuzz_release.version).zip
unzip -DD onefuzz-deployment-$(onefuzz_release.version).zip
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, the issue is that (1) the new files were getting correctly extracted on the build machine, but (2) indirect invocations of azcopy sync were not remotely updating some genuinely new files, because it takes an action/no-action based on local vs. remote timestamps.

If that's true, does that mean we should consider changing the azcopy invocation, e.g. to azcopy copy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe either solution would resolve the issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should use azcopy copy, as unzip -DD isn't portable.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are two different places in deploy.py that invoke the 'sync' command and changing one and not the other could lead to confusion. Any objection to changing both calls?

One is for 'tools' dir and one is for 'instance-specific-setup' dir.

Copy link
Contributor

Choose a reason for hiding this comment

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

Both should be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Brian. I'm happy to go ahead and push the changes to deploy.py. Please let me know if you'd rather do it yourself, but otherwise, I will update.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're welcome to make the updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There appears to be 3 places where azcopy "sync" is used. Should I make all 3 changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

@bmc-msft
Copy link
Contributor

OBE by #347

@bmc-msft bmc-msft closed this Nov 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Apr 17, 2021
@nharper285 nharper285 deleted the user/noharper/yml-update branch October 20, 2021 16:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants