Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ Run these as you would any other R test suite with `devtools::test()`.
A second suite runs integration tests against a live Connect server running locally in Docker.
This has some additional requirements.

- You need a valid Connect license key or file. Put the contents of the license key, or the path to the license file, in the `RSC_LICENSE` environment variable.
- You need a valid Connect license file (`.lic` file). Place it in the root of the repository as `connect-license.lic`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

.gitignore already covers *.lic :)

- You need Docker.
- If you're running on an ARM (non-Intel) Mac, `export DOCKER_DEFAULT_PLATFORM=linux/amd64`
- Run `connectapi:::build_test_env()` to set up the Connect processes in docker
- Run `connectapi:::build_test_env(connect_license_path = "connect-license.lic")` to set up the Connect processes in docker
- By default, this will run against a contemporary version of Connect. To test against an older version, set the environment variable `CONNECT_VERSION` to something else and then run `build_test_env()`.
- Set `CONNECTAPI_INTEGRATED=true` in the environment to enable running the integration tests (they're skipped by default).
- Run them with `source("tests/test-integrated.R")`
Expand Down
8 changes: 4 additions & 4 deletions .github/local/test-connect-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ services:
ports:
- 3939
privileged: true
environment:
- RSC_LICENSE
volumes:
- "${RSC_LICENSE_FILE}:/var/lib/rstudio-connect/license.lic"
connect2:
hostname: connect2
image: rstudio/rstudio-connect:${CONNECT_VERSION}
ports:
- 3939
privileged: true
environment:
- RSC_LICENSE
volumes:
- "${RSC_LICENSE_FILE}:/var/lib/rstudio-connect/license.lic"
5 changes: 2 additions & 3 deletions .github/workflows/integration-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ jobs:
env:
CONNECT_VERSION: ${{ matrix.version }}
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
RSC_LICENSE: ${{ secrets.RSC_LICENSE }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This line could be RSC_LICENSE: /tmp/connect.lic

And then you wouldn't need to do line 58 below at all (which is extra good cause >> $GITHUB_ENV should be used sparingly). Though, if it is just hardcoded, maybe this should be hardcoded in the makefile instead?

But also, stepping back and thinking about this: should this be specified at all? Where does it get used, maybe we can get rid of it entirely?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option would be to add this(hardcoded in our CI yamls) as the connect_license argument to

connectapi:::build_test_env()
shell: Rscript {0}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I'd like to keep the flexibility to specify a different license key path if running these functions locally. Passing it directly as an argument to connectapi:::build_test_env() is the route I went.

CONNECT_LICENSE_FILE: ${{ secrets.CONNECT_LICENSE_FILE }}
CONNECTAPI_INTEGRATED: true

Expand All @@ -54,11 +53,11 @@ jobs:
shell: Rscript {0}

- name: Set up license file
run: echo "$CONNECT_LICENSE_FILE" > $RSC_LICENSE
run: echo "$CONNECT_LICENSE_FILE" > /tmp/connect.lic

- name: Setup test environment
run: |
connectapi:::build_test_env()
connectapi:::build_test_env(connect_license_path = "/tmp/connect.lic")
shell: Rscript {0}

- uses: r-lib/actions/check-r-package@v2
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/pkgdown.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ jobs:
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
CONNECT_VERSION: 2024.03.0
RSC_LICENSE: ${{ secrets.RSC_LICENSE }}
CONNECT_LICENSE_FILE: ${{ secrets.CONNECT_LICENSE_FILE }}

steps:
Expand All @@ -37,11 +36,11 @@ jobs:
needs: website

- name: Set up license file
run: echo "$CONNECT_LICENSE_FILE" > $RSC_LICENSE
run: echo "$CONNECT_LICENSE_FILE" > /tmp/connect.lic

- name: Start Connect
run: |
Rscript -e 'connectapi:::build_test_env()'
Rscript -e 'connectapi:::build_test_env(connect_license_path = "/tmp/connect.lic")'

- name: Build site
run: pkgdown::build_site_github_pages(new_process = FALSE, install = FALSE)
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/pr-commands.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ jobs:
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
CONNECT_VERSION: 2024.03.0
RSC_LICENSE: ${{ secrets.RSC_LICENSE }}
CONNECT_LICENSE_FILE: ${{ secrets.CONNECT_LICENSE_FILE }}
CONNECTAPI_INTEGRATED: true
steps:
Expand All @@ -32,11 +31,11 @@ jobs:
needs: coverage

- name: Set up license file
run: echo "$CONNECT_LICENSE_FILE" > $RSC_LICENSE
run: echo "$CONNECT_LICENSE_FILE" > /tmp/connect.lic

- name: Setup test environment
run: |
connectapi:::build_test_env()
connectapi:::build_test_env(connect_license_path = "/tmp/connect.lic")
shell: Rscript {0}

- uses: r-lib/actions/check-r-package@v2
Expand Down
5 changes: 2 additions & 3 deletions .github/workflows/test-coverage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ jobs:
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
CONNECT_VERSION: 2024.03.0
RSC_LICENSE: ${{ secrets.RSC_LICENSE }}
CONNECT_LICENSE_FILE: ${{ secrets.CONNECT_LICENSE_FILE }}
CONNECTAPI_INTEGRATED: true

Expand All @@ -34,11 +33,11 @@ jobs:
needs: coverage

- name: Set up license file
run: echo "$CONNECT_LICENSE_FILE" > $RSC_LICENSE
run: echo "$CONNECT_LICENSE_FILE" > /tmp/connect.lic

- name: Setup test environment
run: |
connectapi:::build_test_env()
connectapi:::build_test_env(connect_license_path = "/tmp/connect.lic")
shell: Rscript {0}

- name: Test coverage
Expand Down
16 changes: 3 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,17 @@ mail-down:

connect-up:
NETWORK=${NETWORK} \
RSC_LICENSE=$(RSC_LICENSE) \
CONNECT_VERSION=$(CONNECT_VERSION) \
docker compose -f inst/ci/test-connect.yml -f .github/local/make-network.yml up -d

connect-down:
NETWORK=${NETWORK} \
docker compose -f inst/ci/test-connect.yml -f .github/local/make-network.yml down

connect-file-up:
NETWORK=${NETWORK} \
RSC_LICENSE=$(RSC_LICENSE) \
RSC_LICENSE_FILE=$(RSC_LICENSE_FILE) \
CONNECT_VERSION=$(CONNECT_VERSION) \
docker compose -f inst/ci/test-connect-lic.yml -f .github/local/make-network.yml up -d

connect-file-down:
connect-down:
NETWORK=${NETWORK} \
docker compose -f inst/ci/test-connect-lic.yml -f .github/local/make-network.yml down

test-env-up:
NETWORK=${NETWORK} \
RSC_LICENSE=$(RSC_LICENSE) \
RSC_LICENSE_FILE=$(RSC_LICENSE_FILE) \
CONNECT_VERSION=$(CONNECT_VERSION) \
docker compose -f .github/local/test-connect-ci.yml -f .github/local/make-network.yml up -d

Expand Down
106 changes: 59 additions & 47 deletions R/utils-ci.R
Original file line number Diff line number Diff line change
@@ -1,33 +1,5 @@
# nocov start

# TODO: A nicer way to execute these system commands...
# - debug output... better error handling... etc.

determine_license_env <- function(license) {
if (fs::file_exists(license) && fs::path_ext(license) == "lic") {
# Docker needs this to be an absolute path
license <- fs::path_abs(license)
cat_line("determine_license: looks like a license file")
return(list(
type = "file",
cmd_params = c(
"-v",
paste0(license, ":/var/lib/rstudio-connect/license.lic")
),
env_params = c(RSC_LICENSE_FILE = license)
))
} else {
cat_line("determine_license: looks like a license key")
return(list(
type = "key",
cmd_params = c(),
env_params = c(
RSC_LICENSE = license
)
))
}
}

version_to_docker_tag <- function(version) {
# Prior to 2022.09.0, the plain version number was the tag
# After, it's "<ubuntu-codename>-<version>"
Expand All @@ -49,27 +21,25 @@ version_to_docker_tag <- function(version) {
}

compose_start <- function(
connect_license = Sys.getenv("RSC_LICENSE"),
connect_license_path,
connect_version,
clean = TRUE
) {
warn_dire("compose_start")
scoped_dire_silence()

stopifnot(nchar(connect_license) > 0)
stopifnot(fs::file_exists(connect_license_path))
connect_license_path <- fs::path_abs(connect_license_path)

license_details <- determine_license_env(connect_license)
compose_file <- switch(
license_details$type,
"file" = "ci/test-connect-lic.yml",
"ci/test-connect.yml"
compose_file_path <- system.file(
"ci/test-connect-lic.yml",
package = "connectapi"
)

compose_file_path <- system.file(compose_file, package = "connectapi")
env_vars <- c(
CONNECT_VERSION = version_to_docker_tag(connect_version),
PATH = Sys.getenv("PATH"),
license_details$env_params
RSC_LICENSE_FILE = connect_license_path
)
# system2 needs a character vector of name=value
env_vars <- paste(names(env_vars), env_vars, sep = "=")
Expand Down Expand Up @@ -105,9 +75,13 @@ compose_find_hosts <- function(prefix) {

containers <- grep(prefix, docker_ps_output, value = TRUE)
ports <- sub(".*0\\.0\\.0\\.0:([0-9]+)->3939.*", "\\1", containers)
container_names <- sub(".*\\s+([^ ]+)$", "\\1", containers)
cat_line(glue::glue("docker: got ports {ports[1]} and {ports[2]}"))

paste0("http://localhost:", ports)
list(
hosts = paste0("http://localhost:", ports),
container_names = container_names
)
}


Expand Down Expand Up @@ -143,7 +117,7 @@ update_renviron_creds <- function(
}

build_test_env <- function(
connect_license = Sys.getenv("RSC_LICENSE"),
connect_license_path,
clean = TRUE,
username = "admin",
password = "admin0",
Expand All @@ -153,16 +127,16 @@ build_test_env <- function(
scoped_dire_silence()

compose_start(
connect_license = connect_license,
connect_license_path = connect_license_path,
clean = clean,
connect_version = connect_version
)

# It was ci_connect before but it's ci-connect on my machine now;
# this is a regex so it will match either
hosts <- compose_find_hosts(prefix = "ci.connect")
container_info <- compose_find_hosts(prefix = "ci.connect")

wait_for_connect_ready <- function(host, timeout = 120) {
wait_for_connect_ready <- function(host, container_name, timeout = 120) {
client <- HackyConnect$new(server = host, api_key = NULL)
start_time <- Sys.time()
last_msg <- start_time
Expand All @@ -175,7 +149,8 @@ build_test_env <- function(
{
res <- client$GET(url = client$server_url("__ping__"), parser = NULL)
httr::status_code(res) == 200
}
},
silent = TRUE
)
if (isTRUE(ok)) {
return(invisible(TRUE))
Expand All @@ -186,21 +161,58 @@ build_test_env <- function(
}
Sys.sleep(1)
}

# Before failing, capture diagnostics
cat_line("Connect did not become ready in time. Capturing diagnostics...")
cat_line(glue::glue("=== Docker logs for {container_name} ==="))
try(
{
logs <- system2(
"docker",
c("logs", "--tail", "100", container_name),
stdout = TRUE,
stderr = TRUE
)
cat(logs, sep = "\n")
},
silent = TRUE
)

cat_line(glue::glue("=== Docker inspect for {container_name} ==="))
try(
{
inspect <- system2(
"docker",
c("inspect", container_name),
stdout = TRUE,
stderr = TRUE
)
cat(inspect, sep = "\n")
},
silent = TRUE
)

stop("Connect did not become ready in time: ", ping_url)
}

wait_for_connect_ready(hosts[1])
wait_for_connect_ready(hosts[2])
wait_for_connect_ready(
container_info$hosts[1],
container_name = container_info$container_names[1]
)
wait_for_connect_ready(
container_info$hosts[2],
container_name = container_info$container_names[2]
)

cat_line("connect: creating first admin...")
a1 <- create_first_admin(
hosts[1],
container_info$hosts[1],
"admin",
"admin0",
"[email protected]"
)
a2 <- create_first_admin(
hosts[2],
container_info$hosts[2],
"admin",
"admin0",
"[email protected]"
Expand Down
17 changes: 0 additions & 17 deletions inst/ci/test-connect.yml

This file was deleted.