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

logs: fix logs.disabled on Windows #17199

Merged
merged 1 commit into from
May 18, 2023
Merged

logs: fix logs.disabled on Windows #17199

merged 1 commit into from
May 18, 2023

Conversation

tgross
Copy link
Member

@tgross tgross commented May 15, 2023

On Windows the executor returns an error when trying to open the NUL device when we pass it os.DevNull for the stdout/stderr paths. Instead of opening the device, use the discard pipe so that we have platform-specific behavior from the executor itself.

Fixes: #17148


Because our E2E tests are a much more realistic exercise of the behavior, I ran the following Windows batch job on our E2E cluster with Windows 2016. The job has log collection enabled:

jobspec
job "example" {

  type = "batch"

  constraint {
    attribute = "${attr.kernel.name}"
    value     = "windows"
  }

  group "group" {

    task "task" {
      driver = "raw_exec"

      template {
        data = <<EOH
$Number = 1..30
$Number | ForEach-Object {
    if ($_ % 5 -eq 0 -and $_ % 3 -eq 0)
    {
        Write-Host "FizzBuzz"
    }
    elseif ($_ % 3 -eq 0)
    {
        Write-Host "Fizz"
    }
    elseif ($_ % 5 -eq 0)
    {
        Write-Host "Buzz"
    }
    else {
        $_
    }
}
  EOH

        destination = "local/factorial.ps1"
      }

      # logs {
      #   disabled = true
      # }

      config {
        command = "powershell"
        args    = ["local/factorial.ps1"]
      }
    }
  }
}

And got the following expected results:

successful run with logs
$ nomad job run example.nomad
==> 2023-05-17T13:52:07-04:00: Monitoring evaluation "bba99715"
    2023-05-17T13:52:07-04:00: Evaluation triggered by job "example"
    2023-05-17T13:52:08-04:00: Allocation "9e724e91" created: node "2d7b2be9", group "group"
    2023-05-17T13:52:08-04:00: Evaluation status changed: "pending" -> "complete"
==> 2023-05-17T13:52:08-04:00: Evaluation "bba99715" finished with status "complete"
$ nomad alloc status 9e7
ID                  = 9e724e91-ad2e-9538-c4f0-587068874fd4
Eval ID             = bba99715
Name                = example.group[0]
Node ID             = 2d7b2be9
Node Name           = EC2AMAZ-3RIPI07
Job ID              = example
Job Version         = 0
Client Status       = complete
Client Description  = All tasks have completed
Desired Status      = run
Desired Description = <none>
Created             = 9s ago
Modified            = 8s ago

Task "task" is "dead"
Task Resources:
CPU        Memory          Disk     Addresses
0/100 MHz  45 MiB/300 MiB  300 MiB

Task Events:
Started At     = 2023-05-17T17:52:08Z
Finished At    = 2023-05-17T17:52:08Z
Total Restarts = 0
Last Restart   = N/A

Recent Events:
Time                       Type        Description
2023-05-17T13:52:08-04:00  Terminated  Exit Code: 0
2023-05-17T13:52:08-04:00  Started     Task started by client
2023-05-17T13:52:07-04:00  Task Setup  Building Task Directory
2023-05-17T13:52:07-04:00  Received    Task received by client
$ nomad alloc logs 9e7
1
2
Fizz
4
Buzz
Fizz
7
8
Fizz
Buzz
11
Fizz
13
14
FizzBuzz
16
17
Fizz
19
Buzz
Fizz
22
23
Fizz
Buzz
26
Fizz
28
29
FizzBuzz

And then I uncommented the logs { disabled = true } block and ran it again, and got the expected results.

successful run without logs
$ nomad alloc status d78
ID                  = d7822543-8548-249f-b522-2e828b43809a
Eval ID             = 6c6c696d
Name                = example.group[0]
Node ID             = 2d7b2be9
Node Name           = EC2AMAZ-3RIPI07
Job ID              = example
Job Version         = 1
Client Status       = complete
Client Description  = All tasks have completed
Desired Status      = run
Desired Description = <none>
Created             = 7s ago
Modified            = 6s ago

Task "task" is "dead"
Task Resources:
CPU        Memory          Disk     Addresses
0/100 MHz  50 MiB/300 MiB  300 MiB

Task Events:
Started At     = 2023-05-17T17:53:43Z
Finished At    = 2023-05-17T17:53:43Z
Total Restarts = 0
Last Restart   = N/A

Recent Events:
Time                       Type        Description
2023-05-17T13:53:43-04:00  Terminated  Exit Code: 0
2023-05-17T13:53:43-04:00  Started     Task started by client
2023-05-17T13:53:42-04:00  Task Setup  Building Task Directory
2023-05-17T13:53:42-04:00  Received    Task received by client

$ nomad alloc logs d78
Failed to read stdout file: error reading file: Unexpected response code: 404 (log entry for task "task" and log type "stdout" not found)

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

LGTM! Just noting there is no changelog entry or backport labels, if we wanted those before merging.

On Windows the executor returns an error when trying to open the `NUL` device
when we pass it `os.DevNull` for the stdout/stderr paths. Instead of opening the
device, use the discard pipe so that we have platform-specific behavior from the
executor itself.

Fixes: #17148
@tgross tgross force-pushed the b-windows-disable-logging branch from 49a244a to 2124c74 Compare May 18, 2023 12:48
@tgross
Copy link
Member Author

tgross commented May 18, 2023

Changelog fixed. Will merge and backport once that's ✔️

@tgross tgross added backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line and removed backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line labels May 18, 2023
@tgross tgross merged commit 7fc37fa into main May 18, 2023
@tgross tgross deleted the b-windows-disable-logging branch May 18, 2023 13:14
@tgross
Copy link
Member Author

tgross commented May 18, 2023

Only backporting to 1.5.x because 1.4.x and 1.3.x didn't get the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

logs.disabled = true does not work with raw_exec on Windows
2 participants