Skip to content

Commit

Permalink
Add setting for --log-dir flag
Browse files Browse the repository at this point in the history
  • Loading branch information
code-asher committed Jul 19, 2024
1 parent dc93f2d commit fc241b8
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 25 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
### Added

- Extra logging around the IDE spawn to help debugging.
- Add setting to enable logging connection diagnostics from the Coder CLI for
debugging connectivity issues.

## 2.13.0 - 2024-07-16

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import net.schmizz.sshj.common.SSHException
import net.schmizz.sshj.connection.ConnectionException
import org.zeroturnaround.exec.ProcessExecutor
import java.net.URI
import java.nio.file.Path
import java.time.Duration
import java.time.LocalDateTime
import java.time.format.DateTimeFormatter
Expand Down Expand Up @@ -239,6 +240,11 @@ class CoderRemoteConnectionHandle {
return
}

// Makes sure the ssh log directory exists.
if (settings.sshLogDirectory.isNotBlank()) {
Path.of(settings.sshLogDirectory).toFile().mkdirs()
}

// Make the initial connection.
indicator.text = "Connecting ${workspace.ideName} client..."
logger.info("Connecting ${workspace.ideName} client to coder@${workspace.hostname}:22")
Expand Down Expand Up @@ -286,7 +292,7 @@ class CoderRemoteConnectionHandle {
}
// Kill the lifetime if the client is closed by the user.
handle.clientClosed.advise(lifetime) {
logger.info("${workspace.ideName} client ${workspace.hostname} closed")
logger.info("${workspace.ideName} client to ${workspace.hostname} closed")
if (lifetime.status == LifetimeStatus.Alive) {
if (continuation.isActive) {
continuation.resumeWithException(Exception("${workspace.ideName} client was closed"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
CoderGatewayBundle.message("gateway.connector.settings.default-url.comment"),
)
}.layout(RowLayout.PARENT_GRID)
row(CoderGatewayBundle.message("gateway.connector.settings.ssh-log-directory.title")) {
textField().resizableColumn().align(AlignX.FILL)
.bindText(state::sshLogDirectory)
.comment(CoderGatewayBundle.message("gateway.connector.settings.ssh-log-directory.comment"))
}.layout(RowLayout.PARENT_GRID)
}
}

Expand Down
11 changes: 9 additions & 2 deletions src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,11 @@ class CoderCLIManager(
"--stdio",
if (settings.disableAutostart && feats.disableAutostart) "--disable-autostart" else null,
)
val proxyArgs = baseArgs + listOfNotNull(if (feats.reportWorkspaceUsage) "--usage-app=jetbrains" else null)
val proxyArgs = baseArgs + listOfNotNull(
if (settings.sshLogDirectory.isNotBlank()) "--log-dir" else null,
if (settings.sshLogDirectory.isNotBlank()) escape(settings.sshLogDirectory) else null,
if (feats.reportWorkspaceUsage) "--usage-app=jetbrains" else null,
)
val backgroundProxyArgs = baseArgs + listOfNotNull(if (feats.reportWorkspaceUsage) "--usage-app=disable" else null)
val extraConfig =
if (settings.sshConfigOptions.isNotBlank()) {
Expand Down Expand Up @@ -368,6 +372,10 @@ class CoderCLIManager(
if (contents != null) {
settings.sshConfigPath.parent.toFile().mkdirs()
settings.sshConfigPath.toFile().writeText(contents)
// The Coder cli will *not* create the log directory.
if (settings.sshLogDirectory.isNotBlank()) {
Path.of(settings.sshLogDirectory).toFile().mkdirs()
}
}
}

Expand Down Expand Up @@ -453,7 +461,6 @@ class CoderCLIManager(
Features()
} else {
Features(
// Autostart with SSH was added in 2.5.0.
disableAutostart = version >= SemVer(2, 5, 0),
reportWorkspaceUsage = version >= SemVer(2, 13, 0),
)
Expand Down
5 changes: 5 additions & 0 deletions src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ open class CoderSettingsState(
open var ignoreSetupFailure: Boolean = false,
// Default URL to show in the connection window.
open var defaultURL: String = "",
// Value for --log-dir.
open var sshLogDirectory: String = "",
)

/**
Expand Down Expand Up @@ -175,6 +177,9 @@ open class CoderSettings(
return null
}

val sshLogDirectory: String
get() = state.sshLogDirectory

/**
* Given a deployment URL, try to find a token for it if required.
*/
Expand Down
4 changes: 4 additions & 0 deletions src/main/resources/messages/CoderGatewayBundle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,7 @@ gateway.connector.settings.default-url.comment=The default URL to set in the \
URL field in the connection window when there is no last used URL. If this \
is not set, it will try CODER_URL then the URL in the Coder CLI config \
directory.
gateway.connector.settings.ssh-log-directory.title=SSH log directory:
gateway.connector.settings.ssh-log-directory.comment=If set, the Coder CLI will \
output extra SSH information into this directory, which can be helpful for \
debugging connectivity issues.
16 changes: 16 additions & 0 deletions src/test/fixtures/outputs/log-dir.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# --- START CODER JETBRAINS test.coder.invalid
Host coder-jetbrains--foo--test.coder.invalid
ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio --log-dir /tmp/coder-gateway/test.coder.invalid/logs --usage-app=jetbrains foo
ConnectTimeout 0
StrictHostKeyChecking no
UserKnownHostsFile /dev/null
LogLevel ERROR
SetEnv CODER_SSH_SESSION_TYPE=JetBrains
Host coder-jetbrains--foo--test.coder.invalid--bg
ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio --usage-app=disable foo
ConnectTimeout 0
StrictHostKeyChecking no
UserKnownHostsFile /dev/null
LogLevel ERROR
SetEnv CODER_SSH_SESSION_TYPE=JetBrains
# --- END CODER JETBRAINS test.coder.invalid
85 changes: 64 additions & 21 deletions src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,14 @@ internal class CoderCLIManagerTest {
val remove: String,
val headerCommand: String = "",
val disableAutostart: Boolean = false,
val features: Features = Features(),
// Default to the most common feature settings.
val features: Features = Features(
disableAutostart = false,
reportWorkspaceUsage = true,
),
val extraConfig: String = "",
val env: Environment = Environment(),
val sshLogDirectory: Path? = null,
)

@Test
Expand All @@ -309,27 +314,26 @@ internal class CoderCLIManagerTest {
).joinToString(System.lineSeparator())
val tests =
listOf(
SSHTest(listOf("foo", "bar"), null, "multiple-workspaces", "blank", features = Features(false, true)),
SSHTest(listOf("foo", "bar"), null, "multiple-workspaces", "blank", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "blank", "append-blank", "blank", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "blank-newlines", "append-blank-newlines", "blank", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "existing-end", "replace-end", "no-blocks", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "existing-end-no-newline", "replace-end-no-newline", "no-blocks", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "existing-middle", "replace-middle", "no-blocks", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "existing-middle-and-unrelated", "replace-middle-ignore-unrelated", "no-related-blocks", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "existing-only", "replace-only", "blank", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "existing-start", "replace-start", "no-blocks", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "no-blocks", "append-no-blocks", "no-blocks", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "no-related-blocks", "append-no-related-blocks", "no-related-blocks", features = Features(false, true)),
SSHTest(listOf("foo-bar"), "no-newline", "append-no-newline", "no-blocks", features = Features(false, true)),
SSHTest(listOf("foo", "bar"), null, "multiple-workspaces", "blank"),
SSHTest(listOf("foo", "bar"), null, "multiple-workspaces", "blank"),
SSHTest(listOf("foo-bar"), "blank", "append-blank", "blank"),
SSHTest(listOf("foo-bar"), "blank-newlines", "append-blank-newlines", "blank"),
SSHTest(listOf("foo-bar"), "existing-end", "replace-end", "no-blocks"),
SSHTest(listOf("foo-bar"), "existing-end-no-newline", "replace-end-no-newline", "no-blocks"),
SSHTest(listOf("foo-bar"), "existing-middle", "replace-middle", "no-blocks"),
SSHTest(listOf("foo-bar"), "existing-middle-and-unrelated", "replace-middle-ignore-unrelated", "no-related-blocks"),
SSHTest(listOf("foo-bar"), "existing-only", "replace-only", "blank"),
SSHTest(listOf("foo-bar"), "existing-start", "replace-start", "no-blocks"),
SSHTest(listOf("foo-bar"), "no-blocks", "append-no-blocks", "no-blocks"),
SSHTest(listOf("foo-bar"), "no-related-blocks", "append-no-related-blocks", "no-related-blocks"),
SSHTest(listOf("foo-bar"), "no-newline", "append-no-newline", "no-blocks"),
if (getOS() == OS.WINDOWS) {
SSHTest(
listOf("header"),
null,
"header-command-windows",
"blank",
""""C:\Program Files\My Header Command\HeaderCommand.exe" --url="%CODER_URL%" --test="foo bar"""",
features = Features(false, true),
)
} else {
SSHTest(
Expand All @@ -338,27 +342,53 @@ internal class CoderCLIManagerTest {
"header-command",
"blank",
"my-header-command --url=\"\$CODER_URL\" --test=\"foo bar\" --literal='\$CODER_URL'",
features = Features(false, true),
)
},
SSHTest(listOf("foo"), null, "disable-autostart", "blank", "", true, Features(true, true)),
SSHTest(listOf("foo"), null, "no-disable-autostart", "blank", "", true, Features(false, true)),
SSHTest(listOf("foo"), null, "no-report-usage", "blank", "", true, Features(false, false)),
SSHTest(
listOf("foo"),
null,
"disable-autostart",
"blank",
"",
true,
Features(
disableAutostart = true,
reportWorkspaceUsage = true,
),
),
SSHTest(listOf("foo"), null, "no-disable-autostart", "blank", ""),
SSHTest(
listOf("foo"),
null,
"no-report-usage",
"blank",
"",
true,
Features(
disableAutostart = false,
reportWorkspaceUsage = false,
),
),
SSHTest(
listOf("extra"),
null,
"extra-config",
"blank",
extraConfig = extraConfig,
features = Features(false, true),
),
SSHTest(
listOf("extra"),
null,
"extra-config",
"blank",
env = Environment(mapOf(CODER_SSH_CONFIG_OPTIONS to extraConfig)),
features = Features(false, true),
),
SSHTest(
listOf("foo"),
null,
"log-dir",
"blank",
sshLogDirectory = tmpdir.resolve("ssh-logs"),
),
)

Expand All @@ -372,6 +402,7 @@ internal class CoderCLIManagerTest {
dataDirectory = tmpdir.resolve("configure-ssh").toString(),
headerCommand = it.headerCommand,
sshConfigOptions = it.extraConfig,
sshLogDirectory = it.sshLogDirectory?.toString() ?: "",
),
sshConfigPath = tmpdir.resolve(it.input + "_to_" + it.output + ".conf"),
env = it.env,
Expand All @@ -395,12 +426,24 @@ internal class CoderCLIManagerTest {
.replace(newlineRe, System.lineSeparator())
.replace("/tmp/coder-gateway/test.coder.invalid/config", escape(coderConfigPath.toString()))
.replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", escape(ccm.localBinaryPath.toString()))
.let { conf ->
if (it.sshLogDirectory != null) {
conf.replace("/tmp/coder-gateway/test.coder.invalid/logs", it.sshLogDirectory.toString())
} else {
conf
}
}

// Add workspaces.
ccm.configSsh(it.workspaces.toSet(), it.features)

assertEquals(expectedConf, settings.sshConfigPath.toFile().readText())

// SSH log directory should have been created.
if (it.sshLogDirectory != null) {
assertTrue(it.sshLogDirectory.toFile().exists())
}

// Remove configuration.
ccm.configSsh(emptySet(), it.features)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ internal class CoderSettingsTest {
val tmp = Path.of(System.getProperty("java.io.tmpdir"))
val url = URL("http://test.deployment.coder.com")
val dir = tmp.resolve("coder-gateway-test/test-default-token")
var env =
val env =
Environment(
mapOf(
"CODER_CONFIG_DIR" to dir.toString(),
Expand Down Expand Up @@ -386,6 +386,7 @@ internal class CoderSettingsTest {
disableAutostart = getOS() != OS.MAC,
setupCommand = "test setup",
ignoreSetupFailure = true,
sshLogDirectory = "test ssh log directory",
),
)

Expand All @@ -399,5 +400,6 @@ internal class CoderSettingsTest {
assertEquals(getOS() != OS.MAC, settings.disableAutostart)
assertEquals("test setup", settings.setupCommand)
assertEquals(true, settings.ignoreSetupFailure)
assertEquals("test ssh log directory", settings.sshLogDirectory)
}
}

0 comments on commit fc241b8

Please sign in to comment.