Issue 6077 - Provide a Development Container #6078

slominskir · 2024-02-06T20:37:34Z

Bug Description:
It would be nice if we had a quick, easy, repeatable, and portable way to build and run this project - specifically in a container.

Fix Description:
A container specifically intended for getting contributors up and running quickly from anywhere so they can test their PRs.

Fixes: #6077

vashirov

Hi @slominskir,

I thought that the devcontainer definitions should live under .devcontainer, at least according to its specification: https://containers.dev/implementors/spec/

See also:
https://code.visualstudio.com/docs/devcontainers/create-dev-container
and
https://docs.github.com/en/codespaces/setting-up-your-project-for-codespaces/adding-a-dev-container-configuration/introduction-to-dev-containers

This would allow a seamless integration of this container in VSCode and github.dev.

Could you please update your PR according to the spec?
Thanks.

vashirov · 2024-02-07T11:53:27Z

Containerfile-dev

@@ -0,0 +1,9 @@
+FROM quay.io/389ds/ci-images:fedora


This image is x86_64 only. On Apple silicon Macs it would be better to use aarch64 images. The Containerfile for this image is rather small, so I think it would be better to incorporate it in its entirety here. Fedora image is multiarch.

Will do. Also, I was wondering if the container build should be included in the default run instructions. For the most flexibility I suppose it should. For faster start-up it may be best to make it optional and default to using a pre-built image on DockerHub or quay.io (389ds account?). Also to be considered is how to run the container build. I generally have used GitHub action on publish new release trigger. I have generally made the default compose behavior be to use an image and provide an optional build (via compose file precedence) which then requires merging two of three files: compose.yaml, compose.override.yaml, and build.yaml (example, long winded explanation). Not clear how you want this handled.

I've added quay.io/389ds/devcontainer image based on rawhide, it has 2 arches (amd64 and arm64) and is rebuilt nightly. This is enough to open a workspace in a devcontainer in VSCode. devcontainer.json covers some aspects of the compose (ports, mounts, etc), so not sure if it's really needed for this purpose.

Containerfile-dev

slominskir · 2024-02-07T18:42:35Z

Could you please update your PR according to the spec?

Will do. I found the spec to be mostly icing as 90% of the value of a development container is just use a plain container. The spec adds metadata that would allow running directly in a web browser on GitHub.com though so I guess that's something. IntelliJ and VSCode will also integrate with it more readily. It may be the case I'm using Docker Compose at the moment to handle some features handled by spec compliant IDEs.

vashirov

I found the spec to be mostly icing as 90% of the value of a development container is just use a plain container.

I'd rather not reinvent the wheel, given the devcontainer spec is widely supported https://containers.dev/supporting

The spec adds metadata that would allow running directly in a web browser on GitHub.com though so I guess that's something. IntelliJ and VSCode will also integrate with it more readily. It may be the case I'm using Docker Compose at the moment to handle some features handled by spec compliant IDEs.

Yes, devcontainer.json covers aspects of Compose, so a separate YAML file for it might not be needed.

Containerfile-dev

vashirov · 2024-02-08T09:47:16Z

Containerfile-dev

@@ -0,0 +1,9 @@
+FROM quay.io/389ds/ci-images:fedora


I've added quay.io/389ds/devcontainer image based on rawhide, it has 2 arches (amd64 and arm64) and is rebuilt nightly. This is enough to open a workspace in a devcontainer in VSCode. devcontainer.json covers some aspects of the compose (ports, mounts, etc), so not sure if it's really needed for this purpose.

vashirov · 2024-02-08T09:49:32Z

dev.yaml

@@ -0,0 +1,21 @@
+services:


We'd also need SYS_PTRACE capability to run debugger or ASAN builds inside the container. This can be added in devcontainer.json too.

Where is the Dockerfile for https://quay.io/repository/389ds/devcontainer? Should it be stored in an accessible place? If I remove "dnf install libasan" the build fails with:

/usr/bin/ld: cannot find /usr/lib64/libasan.so.8.0.0: No such file or directory collect2: error: ld returned 1 exit status make[1]: *** [Makefile:5801: libsvrcore.la] Error 1 make[1]: Leaving directory '/389-ds-base' make: *** [Makefile:3946: all] Error 2

Seems like it isn't really optional if programmatically running the build.

Similarly, the "dscontainer -r" command fails without "nss-tools" with:

File "/usr/lib64/python3.12/subprocess.py", line 1026, in __init__ self._execute_child(args, executable, preexec_fn, close_fds, File "/usr/lib64/python3.12/subprocess.py", line 1953, in _execute_child raise child_exception_type(errno_num, err_msg, err_filename) FileNotFoundError: [Errno 2] No such file or directory: '/usr/bin/certutil'

Similar for "openssl"

Ah, sorry, it's here: https://github.com/389ds/ds-container/tree/main/devcontainer. Prob should add libasan, openssl, and nss-tools there?

Also, IntelliJ won't launch the devcontainer unlesss ps is installed. Then it will try to copy a backend app into the container. Very buggy at the moment though. So probably need:

dnf install procps in the Containerfile as well.

Bind mounting the project files from the container into the local filesystem for the IDE to edit works reasonably well so I'm not sure I'd use the remote editing IDE feature at the moment. Currently with IntelliJ it opens a new window and kinda works but complains that Java isn't installed inside the container:

Also probably bad to do both at the same time. Either need to remove bind mount else don't remote edit. Not sure if that means devcontainer author must choose up front as if volume is defined in compose it gets used.

Testing running the .devcontainer in GitHub codespaces fails at the moment. At first glance it appears the version of Compose inside codespaces is too old (< 2.24.0) because the dev.yaml compose file appears to choke it with log output:

validating /var/lib/docker/codespacemount/workspace/389-ds-base/dev.yaml: services.dirsrv.env_file.0 must be a string 2024-02-15 18:21:02.402Z: Exit code 15 2024-02-15 18:21:02.473Z: {"outcome":"error","message":"Command failed: docker-compose -f /var/lib/docker/codespacemount/workspace/389-ds-base/dev.yaml -f /var/lib/docker/codespacemount/.persistedshare/docker-compose.codespaces.yml --profile * config","description":"An error occurred retrieving the Docker Compose configuration."} 2024-02-15 18:21:02.477Z: Error: Command failed: docker-compose -f /var/lib/docker/codespacemount/workspace/389-ds-base/dev.yaml -f /var/lib/docker/codespacemount/.persistedshare/docker-compose.codespaces.yml --profile * config 2024-02-15 18:21:02.481Z: at _g (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:462:889) 2024-02-15 18:21:02.486Z: at process.processTicksAndRejections (node:internal/process/task_queues:95:5) 2024-02-15 18:21:02.489Z: at async J$ (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:429:1484) 2024-02-15 18:21:02.491Z: at async x$ (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:409:3165) 2024-02-15 18:21:02.493Z: at async gAA (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:481:3833) 2024-02-15 18:21:02.496Z: at async BC (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:481:4775) 2024-02-15 18:21:02.498Z: at async xeA (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:614:11265) 2024-02-15 18:21:02.500Z: at async UeA (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:614:11006) 2024-02-15 18:21:02.502Z: devcontainer process exited with exit code 1

Could be something else, but running that compose file via Docker Compose works locally with latest Docker Desktop on Windows.

Also worth mentioning codespaces is more of a gee wizz cool thing, but may not be of practical use anyways. Every GitHub user has a very small amount of free codespace time, but as some have observed, once you run out and actually have to pay for it it may end up being cheaper to buy a Macbook laptop and run locally. I'd rather develop locally anyways. Again, why my initial PR ignored the spec. Just being able to run a container locally via compose that is pre-loaded with the correct versions of tools to build and run an app is the real win.

Also worth mentioning our preloaded devcontainer probably should use tini or similar. Example.

This week I will get my hands on Windows 11 laptop to test this. I believe primary use case is still local development, not cloud-first, although it's nice if it's supported.

slominskir · 2024-02-12T14:28:43Z

In response to:

I'd rather not reinvent the wheel
...
Yes, devcontainer.json covers aspects of Compose, so a separate YAML file for it might not be needed.

Actually, devcontainer.json is the one re-inventing the wheel. Docker Compose has been around a lot longer and devcontainer spec just copies much of the same information into a different format. This is why my initial PR ignored the spec. It looks to me like Microsoft applying a thin wrapper around existing Docker APIs, and isn't really that well supported either (mostly Microsoft VS Code and GitHub). For example if I try to create a DevContainer in IntelliJ I see it is labeled as a Beta feature. When developing with containers it often involves orchestrating multiple containers at once, which means it generally is best to use Docker Compose. Fortunately the devspec does allow just deferring to Docker Compose, which is what I recommend:

devcontainer.json

{
  "dockerComposeFile": "dev.yaml",
  "service": "dirsrv",
  "workspaceFolder": "/389-ds-base"
}

It's also possible to embed this thin wrapper metadata into the Docker image itself using Docker image labels: https://containers.dev/implementors/spec/#image-metadata

vashirov · 2024-02-20T12:36:03Z

.devcontainer/devcontainer.json

@@ -0,0 +1,5 @@
+{
+  "dockerComposeFile": "../dev.yaml",


Preferred name for Compose file is compose.yaml: https://docs.docker.com/compose/compose-application-model/#the-compose-file
Please also move it under .devcontainer.

Done. The compose.yaml name is the default (doesn't require -f) and when it was at root of project I wasn't sure the dev compose was the "default" as you might have a demo of the project as the default compose (that's what I do). Moving to .devcontainer makes it a non-issue.

vashirov · 2024-02-20T12:36:30Z

Containerfile-dev

+FROM quay.io/389ds/devcontainer
+ARG CUSTOM_CRT_URL
+
+ARG CUSTOM_CRT_URL


ARG is added twice, typo?

Typo. It might make sense to just move everything to upstream container build that can be moved there (tini, default envs) and then move site-specific cert handling to a runtime entrypoint. Proposed changes are now in place. Since upstream Containerfile is in separate project I updated the Containerfile here, but this Containerfile can probably go away if we move all this upstream.

vashirov · 2024-02-20T12:36:55Z

Containerfile-dev

+          curl -o /etc/pki/ca-trust/source/anchors/customcert.crt $CUSTOM_CRT_URL \
+          && update-ca-trust \
+          ; fi \
+          && dnf install -y openssl nss-tools libasan procps


I've updated devcontainer image, it now contains these packages.

Great. Consider setting the environment variable default values in the upstream Containerfile as well. Plus consider setting the entrypoint to use tini with container-entrypoint.sh that handles site-specific cert handling.

vashirov · 2024-02-20T12:37:16Z

DEV_CONTAINER.md

Please move it under .devcontainer/README.md.

vashirov · 2024-02-20T12:37:45Z

DEV_CONTAINER.md

+| CFLAGS           | C compiler flags                                      | "-O2 -g"          |
+| CXXFLAGS         | C++ compiler flags                                    |                   |
+| LDFLAGS          | Linker flags                                          |                   |
+| DS_BACKEND_NAME  | Development database backend                          | example           |


I don't see this env variable used anywhere in the source.

The backend name is needed as soon as you want to create a database, but otherwise not strictly required. I use it in the default setup scripts in my dirsrv container here:

https://github.com/JeffersonLab/dirsrv/blob/ed4b8d21988cfc42004f4902ddea9c3768219585/scripts/example/docker-entrypoint-initdb.d/01_schema.sh#L5

vashirov · 2024-02-20T12:41:31Z

DEV_CONTAINER.md

+# 389 Directory Server Development Container
+
+## What is this for?
+The Development Container simplifies building and running the 389 Directory Server from anywhere including Windows, Mac, and Linux by leveraging a container.  The environment is therefore already setup with the correct tools and tool versions to build and run. 


Nit: please avoid double/triple spaces after dot.

Fixed. This is interesting. The number of spaces is somewhat subjective but matching project README (singled space after period in the two sentences that follow another) makes sense (consistency is prob most important). When I was in school we had to use two spaces (since middle school typing class). It's changed over the years, but appears to have always been somewhat subjective and guide specific. I think originally two spaces is a typewriter convention due to monospaced fonts. When the README is translated to HTML all spaces are collapsed in browser per HTML rules anyways, but source code formatting is important. It looks like somewhat recently (2019) APA style guide folded to pressure and changed from two spaces to 1 and then in 2020 Microsoft updated Word to ensure single spaces. I have like 25+ years of typing habit/reflex to undo now I guess. Also Markdown requires trailing 2 spaces on last sentence in a paragraph if your intention is to create a newline as an actual newline character doesn't cut it (at first I thought your comment was about this).

https://www.theatlantic.com/science/archive/2018/05/two-spaces-after-a-period/559304/

https://www.grammarly.com/blog/spaces-after-period/

vashirov · 2024-02-20T12:42:43Z

dev-defaults.env

+LDFLAGS=""
+DS_DM_PASSWORD=password
+DS_SUFFIX_NAME=dc=example,dc=com
+DS_BACKEND_NAME=example


I don't see this env variable used anywhere in the source.

vashirov · 2024-02-20T12:45:25Z

dev.yaml

+    cap_add:
+      - SYS_PTRACE
+    env_file:
+      - path: ./dev-defaults.env


Please move these env files under .devcontainer, and omit dev-. I think it's clear that this is a devcontainer, no need to put Dev/Development everywhere.

vashirov · 2024-02-20T12:47:44Z

dev.yaml

@@ -0,0 +1,21 @@
+services:


This week I will get my hands on Windows 11 laptop to test this. I believe primary use case is still local development, not cloud-first, although it's nice if it's supported.

vashirov · 2024-02-20T12:48:09Z

Containerfile-dev

Please move this file under .devcontainer/Containerfile.

slominskir · 2024-02-20T16:24:02Z

Many things can be moved to the upstream build or otherwise out of Compose, but environment overrides are tricky. The Compose optional env_file doesn't appear to have an equivalent, but that relatively new feature also doesn't work on GitHub (presumably Compose version is too old). A devcontainer JSON metadata that just pointed at an image and skipped Compose is almost the same except the override.env file is no longer optional, but required: https://github.com/389ds/389-ds-base/pull/6078/files#diff-66406abc8095b6e7bc6eb42880d7826f4dfe8af41e5f319b9b617b76adaa5ff1.

Might be possible to commit an empty file then afterwards add file to .gitignore so that user changes later aren't picked up. An empty file would satisfy the no way to mark it optional issue.

slominskir · 2024-02-20T18:03:33Z

Also worth mentioning the local bind mount approach doesn't work on GitHub codespaces either. Only the remote development option works (instruments the dev container with an agent that communicates via ports). See: https://code.visualstudio.com/remote/advancedcontainers/add-local-file-mount. So it seems decision to support GitHub codespaces (cloud dev) may dictate how we configure dev container. Having an alternative .devcontainer metadata file might be an option as some of the decisions such as bind local files vs remote dev and container image vs container build could be made options by having separate metadata files.

Also worth mentioning despite "dev container" word being squatted on by Microsoft here it really doesn't represent all development containers and certainly isn't only way to make them. I think we're still in the big companies duking it out for control phase. There are a ton of competing specs / companies (with admittedly less on the nose sounding tech names) such as:

https://www.gitpod.io/ (GitLab, Amazon)
https://coder.com/ (Netflix, DropBox, Square) - does actually support .devcontainer json now, but native format is templates
https://devpod.sh/ - open source version of codespaces?
https://github.com/recode-sh/cli - this one probably dying, but it looks like another open source version of codespaces basically

Bug Description: It would be nice if we had a quick, easy, repeatable, and portable way to build and run this project - specifically in a container. Fix Description: A container specifically intended for getting contributors up and running quickly from anywhere so they can test their PRs. Fixes: 389ds#6077

slominskir · 2024-02-20T20:27:44Z

So the empty overrides.env file isn't so great because now Git tracks changes to it even though it's in .gitignore. It may be ok to remove it and the runArgs from the devcontainer.json for the cloud run scenario and instead just have users override the default environment at runtime in-shell if desired (the env vars are mostly for the build commands and dscontainer command). For testing and dev the defaults probably won't be changed much anyways.

I set the default devcontainer.json to be the cloud use-case (no local files mounted) as GitHub codespaces doesn't appear to give you an option to choose the file. For local dev (with local file mount) you don't really need the devcontainer wrapper metadata and instead can just issue docker compose up. So maybe both use-cases are supported this way.

Users can avoid spending hours trying to get the environment setup and can instead either issue docker compose up else click on the Code button in GitHub to run the dev container in the cloud and view it in-browser. Verified the build works, though GitHub starts to sweat and warns you with:

Not sure how long the free/personal 120 processor core-hours will last at this rate!

I was also able to run /usr/libexec/dirsrv/dscontainer -r & and then /usr/libexec/dirsrv/dscontainer -s, so it seems you can run the app too, though the debugging shows some warning that may be related to running in cloud container:

INFO: 389-ds-container started.
[20/Feb/2024:20:22:24.896411080 +0000] - ERR - SECU_Strerror - Sequence error in error strings at item -4985 - error -4992 (Paged Search Time Limit Exceeded - T3)
[20/Feb/2024:20:22:24.911722854 +0000] - ERR - SECU_Strerror - Should come after 393 - error -4992 (Plugin - P1)

vashirov · 2024-02-21T09:42:00Z

Also worth mentioning despite "dev container" word being squatted on by Microsoft here it really doesn't represent all development containers and certainly isn't only way to make them.

It appears that you and I have different interpretations of what "devcontainer" is. Some of your proposed changes are irrelevant to our team workflows and are suitable only for your needs. In this case, I suggest that you create a custom compose.yaml file and use your own container images to meet your specific requirements. This will allow you to have more control over your development environment and tailor it to your specific needs.

At this time, I will stop working on this and will be prioritizing the fixing of our broken CI (as you keenly pointed out in #6102).

Thanks for understanding.

slominskir · 2024-02-21T14:46:05Z

Hi,
I'm not sure where our differences lie in understanding dev containers, please spell them out. I also don't see anything specific to my needs, please elaborate.

My perspective is that a dev container is simply a container used for development and is important to make it easy for contributors. I have pointed out that there are at least two very different workflows: 1 in the cloud and 2 locally. I think the local workflow is more important, but the cloud one is cool. I've come up with a solution that addresses both. For the local workflow I believe a Compose metadata wrapper/runner/orchestrator is the best case. For the cloud version Microsoft's .devcontainer spec appears reasonable.

I have already created a dev container, and used it to test my PRs. Ideally one is simply provided by the project. Asking contributors to spend hours getting their environment right is an unnecessary hurdle nowadays. I hope you push this across the finish line eventually.

Regards,

Ryan

vashirov requested changes Feb 7, 2024

View reviewed changes

vashirov reviewed Feb 8, 2024

View reviewed changes

slominskir force-pushed the dev-container branch from 4628301 to 0bcfd69 Compare February 12, 2024 14:13

vashirov requested changes Feb 20, 2024

View reviewed changes

slominskir force-pushed the dev-container branch from 7a1ad80 to c67527d Compare February 20, 2024 19:43

slominskir added 2 commits February 20, 2024 14:53

Default to remote dev mode to make cloud services happy

29b967a

Attempting to add empty ignored file to act as default for cloud service

97c320f

Issue 6077 - Provide a Development Container #6078

Are you sure you want to change the base?

Issue 6077 - Provide a Development Container #6078

Conversation

slominskir commented Feb 6, 2024

vashirov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slominskir Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slominskir commented Feb 7, 2024

vashirov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slominskir Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slominskir Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slominskir commented Feb 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slominskir Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slominskir commented Feb 20, 2024 • edited Loading

slominskir commented Feb 20, 2024 • edited Loading

slominskir commented Feb 20, 2024

vashirov commented Feb 21, 2024

slominskir commented Feb 21, 2024

slominskir Feb 7, 2024 •

edited

Loading

slominskir Feb 12, 2024 •

edited

Loading

slominskir Feb 15, 2024 •

edited

Loading

slominskir Feb 20, 2024 •

edited

Loading

slominskir commented Feb 20, 2024 •

edited

Loading

slominskir commented Feb 20, 2024 •

edited

Loading