Improvements to devcontainer #6923

rynowak · 2023-12-06T19:02:35Z

Engineering Improvement

This PR from @lechnerc77 added a new devcontainer that we can use for developing Radius.

Now that this is a available, we can go a bit further with this.

Area for Improvement (required)

DevContainer

Proposed Changes

(done) ~~Remove the old "samples" devcontainer config here. This was created a long time ago for working with our samples, and can be removed.~~
(done) ~~Update our contributing documentation to highlight and recommend the devcontainer + codespaces for contributing to the repo. This is definitely the easiest way for a new contributor to get started.~~
(@rynowak) Update the list of supported dev-container "collections" here.
(done) Update our contributing documentation here to highlight the existing debug targets we have defined. This docs page was written before we checked in tasks.json. We should document the debugging targets we have defined and update this page, which is stale.
(done) Add additional tools to the devcontainer. We got a good start to this, but some tools are still missing. We should aim to include everything we use regularly in Radius development tasks. This is a good list to start with.

AB#10697

The text was updated successfully, but these errors were encountered:

radius-triage-bot · 2023-12-06T19:02:48Z

👋 @rynowak Thanks for filing this issue.

A project maintainer will review this issue and get back to you soon.

We also welcome community contributions! If you would like to pick this item up sooner and submit a pull request, please visit our contribution guidelines and assign this to yourself by commenting "/assign" on this issue.

For more information on our triage process please visit our triage overview

lechnerc77 · 2023-12-06T20:53:23Z

@rynowak I would be happy to support with the tasks you mentioned. Especially concerning the existing devcontainer I made a proposal in #6906 (comment).

rynowak · 2023-12-06T21:51:13Z

Hey @lechnerc77 that would be awesome if you want to help. We'd be happy to accept contributions for the ideas mentioned above, or any other ideas you come up with 👍

lechnerc77 · 2023-12-07T07:01:59Z

Then I will start with the rad CLI as dev container feature as prerequisite for a dev container setup. After that I will check the tools you mentioned and see how to best get them into a dev container setup

Part-of: radius-project#6923 This change removes our **old** **outdated** and **unused** devcontainer. - There's another devcontainer definition in our repo and it's much more useful for developing Radius. - There's another devcontainer definition in our samples repo that's useful for trying out Radius.

rynowak · 2023-12-07T18:23:52Z

I sent a PR to remove the old devcontainer. #6931

Part-of: radius-project#6923 This change removes our **old** **outdated** and **unused** devcontainer. - There's another devcontainer definition in our repo and it's much more useful for developing Radius. - There's another devcontainer definition in our samples repo that's useful for trying out Radius. Signed-off-by: Ryan Nowak <[email protected]>

# Description This change removes our **old** **outdated** and **unused** devcontainer. - There's another devcontainer definition in our repo and it's much more useful for developing Radius. - There's another devcontainer definition in our samples repo that's useful for trying out Radius. Part-of: #6923  - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional).  ## Auto-generated summary   ### <samp>🤖[[deprecated]](https://githubnext.com/copilot-for-prs-sunset) Generated by Copilot at b102f3f</samp> ### Summary 📱🛠️🌐  Removed unused files related to tutorials-codespace from `.devcontainer` folder. These files were not needed for the radius project and were causing confusion and clutter. > _`is_valid` checks_ > _refactored for clarity_ > _autumn leaves falling_ ### Walkthrough * Remove files related to tutorials-codespace devcontainer ([link](https://github.com/radius-project/radius/pull/6931/files?diff=unified&w=0#diff-90dfdf0cccb76f32d9e89ff71733aabc26f1a0d8d2445bfbc9720619eebee531), [link](https://github.com/radius-project/radius/pull/6931/files?diff=unified&w=0#diff-af7eb013f0d068ff8ab6c1ccc8decd4d0f1e503a38b4f1fae9e585e0093611e0), [link](https://github.com/radius-project/radius/pull/6931/files?diff=unified&w=0#diff-664b774cd22a653cdabb9bb92ac796091b54e2cdd4e897d24d064dc59c1c0765), [link](https://github.com/radius-project/radius/pull/6931/files?diff=unified&w=0#diff-852fe25132b51f24a27f106794582d8c09feb949cdc95e26974d9859f4fd96a1)) Signed-off-by: Ryan Nowak <[email protected]>

radius-triage-bot · 2023-12-11T20:44:13Z

👍 We've reviewed this issue and have agreed to add it to our backlog. Please subscribe to this issue for notifications, we'll provide updates when we pick it up.

We also welcome community contributions! If you would like to pick this item up sooner and submit a pull request, please visit our contribution guidelines and assign this to yourself by commenting "/assign" on this issue.

For more information on our triage process please visit our triage overview

lechnerc77 · 2023-12-13T16:59:16Z

@rynowak to avoid a misunderstanding from my side: the list you referenced above with tools that should be available in the devcontainer should be added to the contributors devcontainer available here, correct?

If yes should we also add/streamline the devcontainer setup from a Radius consumer perspective (probably the one in this repository: https://github.com/radius-project/samples)?

rynowak · 2023-12-13T17:16:20Z

@rynowak to avoid a misunderstanding from my side: the list you referenced above with tools that should be available in the devcontainer should be added to the contributors devcontainer available here, correct?

If I understand the question, then yes I agree.

I should be able to open the contributor devcontainer and run make generate without errors. Right now there's a bunch of prerequisites that are required to run the code generators (the list you linked).

If yes should we also add/streamline the devcontainer setup from a Radius consumer perspective (probably the one in this repository: https://github.com/radius-project/samples)?

I'd be interested to learn what you think can be streamlined there. The devcontainer in the samples repo is designed to for users to try Radius.

lechnerc77 · 2023-12-14T07:05:26Z

Thanks, then I know what to add for the contributor devcontainer.

Concerning the one from the samples repo I would add/change the following:

Replace the scripted installation of Radius CLI and Dapr via the corresponding devcontainer features
Align the extensions and the features (e.g. install python via feature, same for C#/.NET)

lechnerc77 · 2023-12-14T16:47:48Z

PR with update of devcontainer is available with PR #6954

rynowak · 2023-12-14T17:32:34Z

Thanks, then I know what to add for the contributor devcontainer.

Concerning the one from the samples repo I would add/change the following:
* Replace the scripted installation of Radius CLI and Dapr via the corresponding devcontainer features

* Align the extensions and the features (e.g. install python via feature, same for C#/.NET)

I think this makes sense to me overall.

We're using a script to install rad in the devcontainer so that it will make the branch/release being used. See here.

On a branch that's named for a specific release like v0.27 the codespace installs 0.27.X.
On any other branch the codespace installs edge.

I might be missing something, but I don't know of a way to dynamically pass data into a feature like this. If you have a better way in mind I'd be all ears 👍

This was hardcoded in the past and we don't want to go back to that model. eg: changing the version number and checking it in in each release branch.

rynowak · 2023-12-14T17:40:26Z

Also for the samples devcontainer I wonder if we can switch to mcr.microsoft.com/devcontainers/base.

Right now we're using mcr.microsoft.com/devcontainers/universal which includes a lot of technologies we don't routinely use in samples. https://mcr.microsoft.com/en-us/product/devcontainers/universal/about

@AaronCrawfis you have an opinion about the right base? It feels like the tradeoff here is size/speed vs including a long tail of tools that we don't use in our samples. I'd be in favor of migrating from universal -> base and relying on features.

# Description This PR contains an enhancement of the devcontainer definition as described in #6923 based on the setup list from [here](https://github.com/radius-project/radius/tree/main/docs/contributing/contributing-code/contributing-code-prerequisites). All tools and the corresponding extensions are integrated in the devcontainer. The following commands have been tested and get executed without errors: - `make build && make lint` - `make generate` ## Type of change  - This pull request adds or changes features of Radius and has an approved issue (issue link required).  Connected to: #6923 - issue si not yet resolved as update of documentation is missing ## Auto-generated summary   ### <samp>🤖[[deprecated]](https://githubnext.com/copilot-for-prs-sunset) Generated by Copilot at 05c1e92</samp> ### Summary 🛠️🌐📚  Enhance devcontainer configuration for contributors. Add features, extensions, and commands for `dapr` and other languages and tools. Specify memory requirement in `.devcontainer/contributor/devcontainer.json`. > _To make devcontainer more complete_ > _They added features, tools, and cheats_ > _With extensions galore_ > _And commands to explore_ > _And a comment to help and a `hostRequirements` repeat_ ### Walkthrough * Add comment with link to contributing code prerequisites and expand features and extensions for various programming languages and tools in `.devcontainer/contributor/devcontainer.json` ([link](https://github.com/radius-project/radius/pull/6954/files?diff=unified&w=0#diff-935f2b2e55e0ab1017fd1c544b9191e499e90e473ece926f8e65e95ddfbb1b1eL3-R15), [link](https://github.com/radius-project/radius/pull/6954/files?diff=unified&w=0#diff-935f2b2e55e0ab1017fd1c544b9191e499e90e473ece926f8e65e95ddfbb1b1eL14-R28), [link](https://github.com/radius-project/radius/pull/6954/files?diff=unified&w=0#diff-935f2b2e55e0ab1017fd1c544b9191e499e90e473ece926f8e65e95ddfbb1b1eL21-R48)) * Add postCreateCommand to run code generation commands in `.devcontainer/contributor/devcontainer.json` ([link](https://github.com/radius-project/radius/pull/6954/files?diff=unified&w=0#diff-935f2b2e55e0ab1017fd1c544b9191e499e90e473ece926f8e65e95ddfbb1b1eL21-R48)) * Add hostRequirements to specify minimum memory for devcontainer in `.devcontainer/contributor/devcontainer.json` ([link](https://github.com/radius-project/radius/pull/6954/files?diff=unified&w=0#diff-935f2b2e55e0ab1017fd1c544b9191e499e90e473ece926f8e65e95ddfbb1b1eL21-R48)) Signed-off-by: Christian Lechner <[email protected]>

rynowak · 2023-12-14T18:22:28Z

@lechnerc77 - FYI I've set up a prebuild for the container so it's faster in codespaces:

First run here: https://github.com/radius-project/radius/actions/runs/7212965607/job/19651786246

rynowak · 2023-12-14T18:39:28Z

I've updated the status in the issue. Including an item for me to go register our feature officially.

lechnerc77 · 2023-12-15T11:15:04Z

Thanks, then I know what to add for the contributor devcontainer.
Concerning the one from the samples repo I would add/change the following:
* Replace the scripted installation of Radius CLI and Dapr via the corresponding devcontainer features

* Align the extensions and the features (e.g. install python via feature, same for C#/.NET)
I think this makes sense to me overall.

We're using a script to install rad in the devcontainer so that it will make the branch/release being used. See here.

On a branch that's named for a specific release like v0.27 the codespace installs 0.27.X.

On any other branch the codespace installs edge.

I might be missing something, but I don't know of a way to dynamically pass data into a feature like this. If you have a better way in mind I'd be all ears 👍

This was hardcoded in the past and we don't want to go back to that model. eg: changing the version number and checking it in in each release branch.

I missed that one on my quick scan. That's then not doable with the feature as we need to hardcode the version. Then the proposed "approvements" make no sense. Thanks a lot for the detailed explanation of the setup.

lechnerc77 · 2023-12-15T11:15:50Z

@lechnerc77 - FYI I've set up a prebuild for the container so it's faster in codespaces:
First run here: https://github.com/radius-project/radius/actions/runs/7212965607/job/19651786246

That's perfect, wanted to ask for that, because the container has quite some stuff in it

lechnerc77 · 2023-12-17T12:50:14Z

@rynowak : I tried a first update of the documentation in PR #6965 as mentioned in the issue namely:

Update of contributing documentation to highlight and recommend the devcontainer + codespaces for contributing to the repo.
Update of contributing documentation to highlight the existing debug targets

# Description This updates the devcontainer publishing path to (hopefully): ``` ghcr.io/radius-project/devcontainer-features/radcli:latest ``` This matches the container that everyone seems to be following at: https://containers.dev/collections In particular I don't want the repo name to be part of the path, because we will probably change repos in the future. ## Type of change - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). Part of: #6923 ## Auto-generated summary  copilot:all Signed-off-by: Ryan Nowak <[email protected]>

# Description This PR is a follow-up due to the introduction of dev containers and a new debugging configuration and updates the documentation with the new options. This comprises the: - setup prerequistes - fist commit - debugging This includes the updates of dependent documentation in the "first-commit" directories if applicable. ## Type of change  - This pull request fixes a bug in Radius and has an approved issue (issue link required).  Linked to: #6923 (not closing it, as one more tak is open ) ## Auto-generated summary  copilot:all --------- Signed-off-by: Christian Lechner <[email protected]> Co-authored-by: Ryan Nowak <[email protected]>

rynowak · 2023-12-20T17:31:09Z

Thanks @lechnerc77 - the last remaining task is on me 👍 It's been awesome having your help with this.

lechnerc77 · 2023-12-20T17:54:57Z

@rynowak Thanks for the countless reviews and the feedback, it was a great experience contributing to this project!

rynowak · 2023-12-20T17:57:40Z

Remaining task is out for PR: devcontainers/devcontainers.github.io#339

rynowak added the maintenance Issue is a non-user-facing task like updating tests, improving automation, etc.. label Dec 6, 2023

rynowak mentioned this issue Dec 7, 2023

Remove stale devcontainer #6931

Merged

shalabhms added the triaged This issue has been reviewed and triaged label Dec 11, 2023

shalabhms assigned lechnerc77 Dec 11, 2023

lechnerc77 mentioned this issue Dec 14, 2023

chore: improve setup of devconatiner for contributions #6954

Merged

lechnerc77 mentioned this issue Dec 17, 2023

docs: update prerequiste and debug docs #6965

Merged

rynowak mentioned this issue Dec 18, 2023

Update devcontainer publishing path #6967

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to devcontainer #6923

Improvements to devcontainer #6923

rynowak commented Dec 6, 2023 •

edited

Loading

radius-triage-bot bot commented Dec 6, 2023

lechnerc77 commented Dec 6, 2023

rynowak commented Dec 6, 2023

lechnerc77 commented Dec 7, 2023

rynowak commented Dec 7, 2023

radius-triage-bot bot commented Dec 11, 2023

lechnerc77 commented Dec 13, 2023

rynowak commented Dec 13, 2023

lechnerc77 commented Dec 14, 2023

lechnerc77 commented Dec 14, 2023

rynowak commented Dec 14, 2023

rynowak commented Dec 14, 2023

rynowak commented Dec 14, 2023

rynowak commented Dec 14, 2023

lechnerc77 commented Dec 15, 2023

lechnerc77 commented Dec 15, 2023

lechnerc77 commented Dec 17, 2023

rynowak commented Dec 20, 2023

lechnerc77 commented Dec 20, 2023

rynowak commented Dec 20, 2023

Improvements to devcontainer #6923

Improvements to devcontainer #6923

Comments

rynowak commented Dec 6, 2023 • edited Loading

Engineering Improvement

Area for Improvement (required)

Proposed Changes

radius-triage-bot bot commented Dec 6, 2023

lechnerc77 commented Dec 6, 2023

rynowak commented Dec 6, 2023

lechnerc77 commented Dec 7, 2023

rynowak commented Dec 7, 2023

radius-triage-bot bot commented Dec 11, 2023

lechnerc77 commented Dec 13, 2023

rynowak commented Dec 13, 2023

lechnerc77 commented Dec 14, 2023

lechnerc77 commented Dec 14, 2023

rynowak commented Dec 14, 2023

rynowak commented Dec 14, 2023

rynowak commented Dec 14, 2023

rynowak commented Dec 14, 2023

lechnerc77 commented Dec 15, 2023

lechnerc77 commented Dec 15, 2023

lechnerc77 commented Dec 17, 2023

rynowak commented Dec 20, 2023

lechnerc77 commented Dec 20, 2023

rynowak commented Dec 20, 2023

rynowak commented Dec 6, 2023 •

edited

Loading