Adding support to set `WorkingDirectory` while running a worker #10690

siddharth-ms · 2024-12-11T08:14:22Z

Issue describing the changes in this PR

Logic app needs this supper because PAL worker we are running as part of our PAL project, looks for .spf libraries in either in CurrentDirectory or CurrentDirectory/lib.
So we need a support in functions app to set the working directory of a worker process. Currently it defaults to function app's location.

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

Backporting to the in-proc branch is not required
- Otherwise: Link to backporting PR
My changes do not require documentation changes
- Otherwise: Documentation issue linked to PR
My changes should not be added to the release notes for the next release
- Otherwise: I've added my notes to release_notes.md
My changes do not need to be backported to a previous version
- Otherwise: Backport tracked by issue/PR #issue_or_pr
My changes do not require diagnostic events changes
- Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
I have added all required tests (Unit tests, E2E tests)

Additional information

We have tested the changes using a worker config like below to run a pal process in Linux. Test branch la-hybrid/v4.636.1

{
  "description": {
    "language": "dotnet-isolated",
    "extensions": [
      ".dll"
    ],
    "defaultExecutablePath": "{workerDirectoryPath}lapalfe",
    "executableWorkingDirectory": "{workerDirectoryPath}",
    "defaultWorkerPath": "lapalfe"
  },
  "processOptions": {
    "initializationTimeout": "00:00:30",
    "environmentReloadTimeout": "00:00:30"
  }
}

jviau · 2024-12-11T16:59:41Z

So we need a support in functions app to set the working directory of a worker process. Currently it defaults to function app's location.

Have you tried changing the working directory of your worker app once during startup from within your worker app? https://learn.microsoft.com/en-us/dotnet/api/system.io.directory.setcurrentdirectory?view=net-9.0

siddharth-ms · 2024-12-11T17:20:14Z

So we need a support in functions app to set the working directory of a worker process. Currently it defaults to function app's location.

Have you tried changing the working directory of your worker app once during startup from within your worker app? https://learn.microsoft.com/en-us/dotnet/api/system.io.directory.setcurrentdirectory?view=net-9.0

We are trying to run PAL executable (C++ project coming from a Microsoft Reasearch to run .exe on Linux). Those executable link libraries under cwd or cwd/lib. So the app will not start until it finds and links all dependencies

kshyju · 2024-12-17T01:55:22Z

To clarify, is the goal here to start a Linux executable from the host? If so, where in the environment is it located? Is this a .NET executable, and what is its specific purpose once it starts? For example, how does it initiate the dotnet isolated payload?

Could we leverage the AzureWebJobsScriptRoot environment variable to point to this executable?

siddharth-ms · 2024-12-17T02:15:47Z

To clarify, is the goal here to start a Linux executable from the host? If so, where in the environment is it located? Is this a .NET executable, and what is its specific purpose once it starts? For example, how does it initiate the dotnet isolated payload?

Could we leverage the AzureWebJobsScriptRoot environment variable to point to this executable?

Yes we want to run a linux worker on linux host. The worker will start a windows sandbox and run a .NET Framework exe.
Currently all this info is coming from worker.config.json. Like below:

{
  "description": {
    "language": "dotnet-isolated",
    "extensions": [
      ".dll"
    ],
    "defaultExecutablePath": "{workerDirectoryPath}lapalfe",
    "executableWorkingDirectory": "{workerDirectoryPath}",
    "defaultWorkerPath": "lapalfe"
  },
  "processOptions": {
    "initializationTimeout": "00:00:30",
    "environmentReloadTimeout": "00:00:30"
  }
}

The thing is, PAL, which is published by MS Research expects the dependent libraries to be part of pwd/lib or pwd. Thus we need a mechanism to set the working directory of an executable.

Everything else, as suggested in last meeting, can be done by the configs, and we have removed the flag we added before palEmulated as we don't want SKU specific code in the repo.

Regarding AzureWebjobsScriptRoot: We support multi language workers. So, we run more than one worker in single host. Like a single host can run Java, Js, and other languages as well. Thus, we want to avoid setting it for all the workers through this env var.

jviau · 2024-12-17T18:38:11Z

@siddharth-ms the worker.config.json allows for setting of executable working directing. Is this not sufficient for your scenario? If not, can you explain why?

siddharth-ms · 2024-12-19T10:27:58Z

@siddharth-ms the worker.config.json allows for setting of executable working directing. Is this not sufficient for your scenario? If not, can you explain why?

Ohh, does it? How can I set the working directory?

jviau · 2024-12-19T23:58:41Z

@siddharth-ms the worker.config.json allows for setting of executable working directing. Is this not sufficient for your scenario? If not, can you explain why?

Ohh, does it? How can I set the working directory?

Ah I misread an existing property - nevermind.

mathewc · 2025-01-09T19:09:39Z

src/WebJobs.Script/Workers/ProcessManagement/WorkerDescription.cs

@@ -18,6 +18,11 @@ public abstract class WorkerDescription
        /// </summary>
        public string DefaultExecutablePath { get; set; }

+        /// <summary>
+        /// Gets or sets the working direfory for executing worker.


mathewc · 2025-01-09T19:14:17Z

src/WebJobs.Script/Workers/Rpc/RpcWorkerProcess.cs

@@ -59,7 +61,7 @@ internal RpcWorkerProcess(string runtime,

        internal override Process CreateWorkerProcess()
        {
-            var workerContext = new RpcWorkerContext(Guid.NewGuid().ToString(), RpcWorkerConstants.DefaultMaxMessageLengthBytes, _workerId, _workerProcessArguments, _scriptRootPath, _serverUri);
+            var workerContext = new RpcWorkerContext(Guid.NewGuid().ToString(), RpcWorkerConstants.DefaultMaxMessageLengthBytes, _workerId, _workerProcessArguments, _workerExecutableWorkingDirectory ?? _scriptRootPath, _serverUri);


Consider doing this defaulting in the ctor at the time when you set _workerExecutableWorkingDirectory

mathewc · 2025-01-09T19:16:59Z

test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs

@@ -62,6 +63,23 @@ public RpcWorkerProcessTests()
                testEnv,
                scriptApplicationHostOptions,
                new LoggerFactory());
+
+            _rpcWorkerProcessWithExecutablePath = new RpcWorkerProcess("node",


Any reason why this needs to be a field? Seems like you can just define this locally in your CreateWorkerProcess_AddsExecutableWorkingDirectory test below, since it's not used anywhere else

mathewc · 2025-01-09T19:17:25Z

test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs

@@ -75,7 +93,7 @@ public void Constructor_RegistersHostStartedEvent()
                return eventType == typeof(HostStartEvent);
            };

-            _eventManager.Verify(_ => _.Subscribe(It.Is<IObserver<ScriptEvent>>(p => validate(p))), Times.Once());
+            _eventManager.Verify(_ => _.Subscribe(It.Is<IObserver<ScriptEvent>>(p => validate(p))), Times.AtLeastOnce());


Why this change?

siddharth-ms added 2 commits December 11, 2024 13:25

Integrate PAL Worker

b8a18be

Cleanup the PR

f3cd2bf

siddharth-ms requested review from kshyju and fabiocav December 11, 2024 08:15

siddharth-ms marked this pull request as ready for review December 11, 2024 08:49

siddharth-ms requested a review from a team as a code owner December 11, 2024 08:49

siddharth-ms added 3 commits December 11, 2024 15:12

Fix a bug where executable path updated the app directory env

a7b242e

Adding unit tests

1d6f621

Fix failing tests

ce95d8b

siddharth-ms requested a review from mattchenderson December 11, 2024 12:59

siddharth-ms requested review from mathewc and removed request for mattchenderson December 12, 2024 03:32

siddharth-ms added the pr-in-review label Dec 12, 2024

mathewc reviewed Jan 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support to set `WorkingDirectory` while running a worker #10690

Adding support to set `WorkingDirectory` while running a worker #10690

siddharth-ms commented Dec 11, 2024 •

edited

Loading

jviau commented Dec 11, 2024

siddharth-ms commented Dec 11, 2024

kshyju commented Dec 17, 2024

siddharth-ms commented Dec 17, 2024 •

edited

Loading

jviau commented Dec 17, 2024

siddharth-ms commented Dec 19, 2024

jviau commented Dec 19, 2024

mathewc Jan 9, 2025

mathewc Jan 9, 2025

mathewc Jan 9, 2025

mathewc Jan 9, 2025

Adding support to set WorkingDirectory while running a worker #10690

Are you sure you want to change the base?

Adding support to set WorkingDirectory while running a worker #10690

Conversation

siddharth-ms commented Dec 11, 2024 • edited Loading

Issue describing the changes in this PR

Pull request checklist

Additional information

jviau commented Dec 11, 2024

siddharth-ms commented Dec 11, 2024

kshyju commented Dec 17, 2024

siddharth-ms commented Dec 17, 2024 • edited Loading

jviau commented Dec 17, 2024

siddharth-ms commented Dec 19, 2024

jviau commented Dec 19, 2024

mathewc Jan 9, 2025

Choose a reason for hiding this comment

mathewc Jan 9, 2025

Choose a reason for hiding this comment

mathewc Jan 9, 2025

Choose a reason for hiding this comment

mathewc Jan 9, 2025

Choose a reason for hiding this comment

Adding support to set `WorkingDirectory` while running a worker #10690

Adding support to set `WorkingDirectory` while running a worker #10690

siddharth-ms commented Dec 11, 2024 •

edited

Loading

siddharth-ms commented Dec 17, 2024 •

edited

Loading