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

Update Linux Apt Commands to Handle Lock Timeouts #1861

Merged
merged 51 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
17191bf
Make Command Executor return an object instead of string
nagilson Jun 25, 2024
b34f1a8
Merge branch 'main' into nagilson-fix-eperm
nagilson Jul 1, 2024
330ee2e
Fix Execution Type Parsing
nagilson Jul 1, 2024
ec9fcff
fix linting issue
nagilson Jul 1, 2024
9292386
Fix line parsing logic
nagilson Jul 1, 2024
ab2a7ef
Merge branch 'main' into nagilson-fix-eperm
nagilson Jul 2, 2024
c554a2c
Fix tests
nagilson Jul 2, 2024
a3f0c2b
Run icacls to set execute permissions
nagilson Jul 2, 2024
a7c1c87
move to sha256
nagilson Jul 2, 2024
12753fb
remove unnecessary code
nagilson Jul 2, 2024
eb83ca0
Remove todo that should go in another PR
nagilson Jul 2, 2024
e8ada86
Remove redundant trim
nagilson Jul 2, 2024
05c57a1
rename class CommandProcessorOutput since it didnt match CommandExecutor
nagilson Jul 2, 2024
4f3efdd
Merge branch 'nagilson-fix-eperm' of https://github.com/nagilson/vsco…
nagilson Jul 2, 2024
a75d493
Rename class file
nagilson Jul 2, 2024
29f7e2d
Report json object for failed permissions.
nagilson Jul 2, 2024
cc2ae05
Fix test to use correct folder
nagilson Jul 2, 2024
ba3580f
fix lint
nagilson Jul 2, 2024
3fdb95b
Fix installer file deletion
nagilson Jul 2, 2024
f4b7098
respond to linter
nagilson Jul 3, 2024
6f9ffe5
Try to Add a Timeout for Installer.Exe
nagilson Jul 3, 2024
3fc9fb0
Add .editorconfig for style settings
nagilson Jul 3, 2024
de06ea2
Update Linux Apt Commands to Handle Lock Timeouts
nagilson Jul 3, 2024
cfe0500
Update distro failure message b to have the same information as the o…
nagilson Jul 3, 2024
99a1962
Fix package is available check
nagilson Jul 3, 2024
9064869
Remove verbosity as it is not helpful or needed information to help c…
nagilson Jul 3, 2024
9653d83
Merge remote-tracking branch 'upstream/main' into nagilson-timeout-in…
nagilson Jul 3, 2024
f73efe7
Remove -y
nagilson Jul 3, 2024
6cc6da2
fix json
nagilson Jul 8, 2024
bd18ad1
try forking
nagilson Jul 8, 2024
820b250
just rely on node timeout
nagilson Jul 8, 2024
a6fd742
update test data
nagilson Jul 8, 2024
43f7494
Handle .net installer timeouts
nagilson Jul 8, 2024
03cb9f0
remove unnecessary import
nagilson Jul 8, 2024
ae550cd
Fix tests
nagilson Jul 9, 2024
965c3be
Include stderr for more failure detail
nagilson Jul 9, 2024
b36ff54
fix options check
nagilson Jul 9, 2024
726d3f3
Fix option
nagilson Jul 9, 2024
7be4dea
Include options that are needed to preserve the working env on a gene…
nagilson Jul 9, 2024
29942a8
Remove hash as it already gets anonymized by the regular expression
Jul 9, 2024
5a140bc
Increase lock timeout time as now with waiting for lock it can take l…
Jul 9, 2024
1ac93eb
Add passive option to automate dialog
nagilson Jul 9, 2024
63b7d77
Skip extra hashing thats handled by the regular expression
Jul 9, 2024
f3d979d
Add a timeout to kill our subprocesses that hold onto the lock
Jul 9, 2024
cc182ae
Timeout default is seconds not minutes
Jul 9, 2024
4aed959
Make code more clear to read by moving else after return into else
nagilson Jul 9, 2024
2abd99d
Improve lock file handling on install tracker
nagilson Jul 9, 2024
9fab2cf
Merge branch 'nagilson-timeout-install' into nagilson-fix-linux-lock
nagilson Jul 9, 2024
07fa9c7
Merge remote-tracking branch 'upstream/main' into nagilson-fix-linux-…
nagilson Jul 10, 2024
f056104
Adhere to style convention with release() block
nagilson Jul 10, 2024
b7e8103
add back in -y to automate
nagilson Jul 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions vscode-dotnet-runtime-library/distro-data/distro-support.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,20 @@
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"update",
"||",
"apt-get",
"update"
]
},
{
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"install",
"-y",
"{packageName}"
Expand All @@ -24,6 +31,8 @@
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"remove",
"{packageName}"
]
Expand All @@ -34,13 +43,17 @@
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"update"
]
},
{
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"upgrade",
"-y",
"{packageName}"
Expand All @@ -52,6 +65,8 @@
"runUnderSudo": false,
"commandRoot": "apt-cache",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"search",
"--names-only",
"^{packageName}$"
Expand Down Expand Up @@ -185,6 +200,8 @@
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"install",
"-y",
"wget"
Expand All @@ -211,6 +228,8 @@
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"update"
]
}
Expand All @@ -223,6 +242,8 @@
"runUnderSudo": true,
"commandRoot": "apt-get",
"commandParts": [
"-o",
"DPkg::Lock::Timeout=120",
"update"
]
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env bash
EXECFOLDER=$1 # First argument is the working folder as this is launched with cwd of /root
TIMEOUT_SEC=$2
OKSIGNALFILE="$EXECFOLDER/ok.txt"
COMMANDTORUNFILE="$EXECFOLDER/command.txt"
#OUTPUTFILE="/home/test_output_.txt"
Expand All @@ -19,7 +20,7 @@ do
if test -f "$COMMANDTORUNFILE"; then
# echo "COMMAND FILE FOUND" >> "$OUTPUTFILE" # Leave this here as an example of debugging
COMMAND="$(cat "$COMMANDTORUNFILE" | awk '{$1=$1;print}')"
for validCmd in "${@:2}"
for validCmd in "${@:3}"
do
if [ "$COMMAND" == "$validCmd" ]; then
# Eventually we should split the cmd file to be line by line instead of space separated,
Expand All @@ -31,7 +32,7 @@ do
rm "$COMMANDTORUNFILE"
exit 111777 # Special exit code - arbitrarily picked for when the command is not expected
fi
sudo "${COMMANDARGS[@]}" 2> "$EXECFOLDER/stderr.txt" 1> "$EXECFOLDER/stdout.txt"
timeout $TIMEOUT_SEC sudo "${COMMANDARGS[@]}" 2> "$EXECFOLDER/stderr.txt" 1> "$EXECFOLDER/stdout.txt"
STATUSCODE=$?
echo $STATUSCODE > "$EXECFOLDER/status.txt"
rm "$COMMANDTORUNFILE"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ export class GenericDistroSDKProvider extends IDistroDotnetSDKProvider
command = CommandExecutor.replaceSubstringsInCommands(command, this.missingPackageNameKey, sdkPackage);
const commandResult = (await this.commandRunner.executeMultipleCommands(command))[0];

const noPackageResult = 'no packages found';
return commandResult.stdout.toLowerCase().includes(noPackageResult);
return commandResult.status === '0';
}

public getExpectedDotnetDistroFeedInstallationDirectory(): string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ export abstract class IDistroDotnetSDKProvider {
{
const error = new DotnetAcquisitionDistroUnknownError(new EventBasedError('DotnetAcquisitionDistroUnknownError',
`Automated installation for the distro ${this.distroVersion.distro} is not yet supported.
Please install the .NET SDK manually: https://dotnet.microsoft.com/download`),
Please install the .NET SDK manually: https://dotnet.microsoft.com/download.
If you would like to contribute to the list of supported distros, please visit: https://github.com/dotnet/vscode-dotnet-runtime/blob/main/Documentation/adding-distros.md`),
getInstallFromContext(this.context));
throw error.error;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
DotnetAcquisitionStatusResolved,
DotnetLockAttemptingAcquireEvent,
DotnetLockErrorEvent,
DotnetLockReleasedEvent,
DotnetPreinstallDetected,
DotnetPreinstallDetectionError,
DuplicateInstallDetected,
Expand Down Expand Up @@ -77,26 +78,33 @@ export class InstallTrackerSingleton

protected executeWithLock = async <A extends any[], R>(alreadyHoldingLock : boolean, f: (...args: A) => R, ...args: A): Promise<R> =>
{

const trackingLock = 'tracking.lock';
const lockPath = path.join(__dirname, trackingLock);
fs.writeFileSync(lockPath, '', 'utf-8');

let returnResult : any;

this.eventStream?.post(new DotnetLockAttemptingAcquireEvent(`Lock Acquisition request to begin.`, new Date().toISOString(), lockPath, lockPath));
try
{
if(alreadyHoldingLock)
{
return await f(...(args));
}
const release = await lockfile.lock(lockPath, { retries: { retries: 10, minTimeout: 5, maxTimeout: 2000 } });
try
{
return await f(...(args));
}
finally
else
{
await release();
await lockfile.lock(lockPath, { retries: { retries: 10, minTimeout: 5, maxTimeout: 10000 } })
Copy link
Member

Choose a reason for hiding this comment

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

What units are the min and max timeout in?

Copy link
Member Author

Choose a reason for hiding this comment

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

they are in MS. The library will do an exponential equation where it starts trying again after 5 ms and then takes longer each retry until its at 10000 ms.

.then(async (release) =>
{
returnResult = await f(...(args));
this.eventStream?.post(new DotnetLockReleasedEvent(`Lock about to be released.`, new Date().toISOString(), lockPath, lockPath));
return release();
})
.catch((e : Error) =>
{
// Either the lock could not be acquired or releasing it failed
this.eventStream?.post(new DotnetLockErrorEvent(e, e.message, new Date().toISOString(), lockPath, lockPath));
});
}
}
catch(e : any)
Expand All @@ -105,6 +113,8 @@ export class InstallTrackerSingleton
this.eventStream.post(new DotnetLockErrorEvent(e, e?.message ?? 'Unable to acquire lock to update installation state', new Date().toISOString(), lockPath, lockPath));
throw new EventBasedError('DotnetLockErrorEvent', e?.message, e?.stack);
}

return returnResult;
}

public async clearPromises() : Promise<void>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ export abstract class DotnetLockEvent extends DotnetFileEvent
constructor(public readonly eventMessage: string, public readonly time: string, public readonly lock: string, public readonly file: string) { super(eventMessage, time, file); }

public getProperties() {
return {Message: this.eventMessage, Time: this.time, Lock: TelemetryUtilities.HashData(this.lock), File: TelemetryUtilities.HashData(this.file)};
return {Message: this.eventMessage, Time: this.time, Lock: this.lock, File: this.file};
}
}

Expand All @@ -982,7 +982,7 @@ export class DotnetLockErrorEvent extends DotnetLockEvent {
public readonly eventMessage: string, public readonly time: string, public readonly lock: string, public readonly file: string) { super(eventMessage, time, lock, file); }

public getProperties() {
return {Error: this.error.toString(), Message: this.eventMessage, Time: this.time, Lock: TelemetryUtilities.HashData(this.lock), File: TelemetryUtilities.HashData(this.file)};
return {Error: this.error.toString(), Message: this.eventMessage, Time: this.time, Lock: this.lock, File: this.file};
}

}
Expand Down
12 changes: 7 additions & 5 deletions vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ Please install the .NET SDK manually by following https://learn.microsoft.com/en
const options = { name: `${sanitizedCallerName ?? 'NET Install Tool'}` };

fs.chmodSync(shellScriptPath, 0o500);
exec((`"${shellScriptPath}" "${this.sudoProcessCommunicationDir}" ${this.validSudoCommands?.join(' ')} &`), options, (error?: any, stdout?: any, stderr?: any) =>
const timeoutSeconds = Math.max(100, this.context.timeoutSeconds);
exec((`"${shellScriptPath}" "${this.sudoProcessCommunicationDir}" "${timeoutSeconds}" ${this.validSudoCommands?.join(' ')} &`), options, (error?: any, stdout?: any, stderr?: any) =>
{
this.context?.eventStream.post(new CommandExecutionStdOut(`The process spawn: ${fullCommandString} encountered stdout, continuing
${stdout}`));
Expand Down Expand Up @@ -198,7 +199,7 @@ Please report this at https://github.com/dotnet/vscode-dotnet-runtime/issues.`),

// Lock the directory -- this is not a system wide lock, only a library lock we must respect in the code.
// This will allow the process to still edit the directory, but not our extension API calls from overlapping with one another.
await lockfile.lock(fakeLockFile, { lockfilePath: directoryLockPath, retries: { retries: 10, minTimeout: 5, maxTimeout: 2000 } } )
await lockfile.lock(fakeLockFile, { lockfilePath: directoryLockPath, retries: { retries: 10, minTimeout: 5, maxTimeout: 10000 } } )
.then(async (release: () => void) =>
{
this.context?.eventStream.post(new DotnetLockAcquiredEvent(`Lock Acquired.`, new Date().toISOString(), directoryLockPath, fakeLockFile));
Expand Down Expand Up @@ -269,7 +270,7 @@ It had previously spawned: ${this.hasEverLaunchedSudoFork}.`), getInstallFromCon
// This will allow the process to still edit the directory, but not our extension API calls from overlapping with one another.


await lockfile.lock(fakeLockFile, { lockfilePath: directoryLockPath, retries: { retries: 10, minTimeout : 5, maxTimeout: 2000 } } )
await lockfile.lock(fakeLockFile, { lockfilePath: directoryLockPath, retries: { retries: 10, minTimeout : 5, maxTimeout: 10000 } } )
.then(async (release: () => any) =>
{
this.context?.eventStream.post(new DotnetLockAcquiredEvent(`Lock Acquired.`, new Date().toISOString(), directoryLockPath, fakeLockFile));
Expand Down Expand Up @@ -331,8 +332,9 @@ ${(commandOutputJson as CommandExecutorResult).stderr}`));
if((commandOutputJson as CommandExecutorResult).status !== '0' && failOnNonZeroExit)
{
const err = new CommandExecutionNonZeroExitFailure(new EventBasedError('CommandExecutionNonZeroExitFailure',
`Cancelling .NET Install, as command ${commandToExecuteString} returned with status ${(commandOutputJson as CommandExecutorResult).status}.`),
getInstallFromContext(this.context));
`Cancelling .NET Install, as command ${commandToExecuteString} returned with status ${(commandOutputJson as CommandExecutorResult).status}.
${(commandOutputJson as CommandExecutorResult).stderr}.`),
getInstallFromContext(this.context));
this.context?.eventStream.post(err);
throw err.error;
}
Expand Down
2 changes: 1 addition & 1 deletion vscode-dotnet-runtime-library/src/Utils/FileUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class FileUtilities extends IFileUtilities
if(!alreadyHoldingLock)
{
eventStream?.post(new DotnetLockAttemptingAcquireEvent(`Lock Acquisition request to begin.`, new Date().toISOString(), directoryLockPath, filePath));
await lockfile.lock(filePath, { lockfilePath: directoryLockPath, retries: { retries: 10, minTimeout: 5, maxTimeout: 2000 } } )
await lockfile.lock(filePath, { lockfilePath: directoryLockPath, retries: { retries: 10, minTimeout: 5, maxTimeout: 10000 } } )
.then(async (release) =>
{
eventStream?.post(new DotnetLockAcquiredEvent(`Lock Acquired.`, new Date().toISOString(), directoryLockPath, filePath));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ suite('Linux Distro Logic Unit Tests', () =>
if(shouldRun)
{
const recVersion = await provider.getRecommendedDotnetVersion(installType);
assert.equal(mockExecutor.attemptedCommand, 'apt-cache search --names-only ^dotnet-sdk-9.0$', 'Searched for the newest package last with regex'); // this may fail if test not exec'd first
assert.equal(mockExecutor.attemptedCommand,
'apt-cache -o DPkg::Lock::Timeout=120 search --names-only ^dotnet-sdk-9.0$', 'Searched for the newest package last with regex'); // this may fail if test not exec'd first
// the data is cached so --version may not be executed.
assert.equal(recVersion, '8.0.1xx', 'Resolved the most recent available version : will eventually break if the mock data is not updated');
}
Expand Down Expand Up @@ -151,23 +152,23 @@ Microsoft.NETCore.App 7.0.5 [/usr/lib/dotnet/shared/Microsoft.NETCore.App]`, std
if(shouldRun)
{
await provider.installDotnet(mockVersion, installType);
assert.equal(mockExecutor.attemptedCommand, 'sudo apt-get install -y dotnet-sdk-7.0');
assert.equal(mockExecutor.attemptedCommand, 'sudo apt-get -o DPkg::Lock::Timeout=120 install -y dotnet-sdk-7.0');
}
}).timeout(standardTimeoutTime);

test('Runs Correct Uninstall Command', async () => {
if(shouldRun)
{
await provider.uninstallDotnet(mockVersion, installType);
assert.equal(mockExecutor.attemptedCommand, 'sudo apt-get remove dotnet-sdk-7.0');
assert.equal(mockExecutor.attemptedCommand, 'sudo apt-get -o DPkg::Lock::Timeout=120 remove dotnet-sdk-7.0');
}
}).timeout(standardTimeoutTime);

test('Runs Correct Update Command', async () => {
if(shouldRun)
{
await provider.upgradeDotnet(mockVersion, installType);
assert.equal(mockExecutor.attemptedCommand, 'sudo apt-get upgrade -y dotnet-sdk-7.0');
assert.equal(mockExecutor.attemptedCommand, 'sudo apt-get -o DPkg::Lock::Timeout=120 upgrade -y dotnet-sdk-7.0');
}
}).timeout(standardTimeoutTime*1000);
});