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

contrib: Add devcontainer configuration #22856

Merged
merged 2 commits into from
Jan 26, 2023

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Dec 23, 2022

Description

This is to provide the quick way for new contributors to bootstrap
development environment with devcontainer, which is
widely used in vscode as well as github codespaces. The
docker image is completely re-used from cilium builder image.

Minimum set of features (e.g. docker-in-docker) is enabled by default.
While enabling more features is handy, it will increase the bootstrap
time. Hence, only docker is enabled as required by Makefile targets (e.g
helm-docs).

Signed-off-by: Tam Mach tam.mach@cilium.io

Testing

Please find below the snippet of local devcontainer with vscode

root@1a0f5e8517c5:/go/src/github.com/cilium/cilium# make dev-doctor
go version 2>/dev/null || ( echo "go not found, see https://golang.org/doc/install" ; false )
go version go1.19.3 linux/amd64
go run ./tools/dev-doctor
RESULT    CHECK           MESSAGE
ok        os/arch         linux/amd64
ok        uname           Linux 1a0f5e8517c5 6.0.12-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Dec 8 17:15:53 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
ok        root-dir        found /go/src/github.com/cilium/cilium
ok        make            found /usr/bin/make, version 4.3
ok        go              found /usr/local/go/bin/go, version 1.19.3
warning   tparse          tparse not found in $PATH
ok        clang           found /usr/local/bin/clang, version 10.0.0
ok        docker-server   found /usr/bin/docker, version 20.10.22
ok        docker-client   found /usr/bin/docker, version 20.10.22
ok        docker-buildx   found /usr/bin/docker, version 0.9.1
warning   ginkgo          ginkgo not found in $PATH
warning   golangci-lint   golangci-lint not found in $PATH
ok        docker          found /usr/bin/docker, version 20.10.22
warning   helm            helm not found in $PATH
ok        llc             found /usr/local/bin/llc, version 10.0.0
info      vagrant         vagrant not found in $PATH
info      virtualbox      virtualbox not found in $PATH
info      vboxheadless    vboxheadless not found in $PATH
warning   pip3            pip3 not found in $PATH
warning   cfssl           cfssl not found in $PATH
warning   cfssljson       cfssljson not found in $PATH
ok        docker-group    user root in docker group

CHECK           HINT
tparse          Run "go install github.com/mfridman/tparse@latest"
ginkgo          Run "go install github.com/onsi/ginkgo/ginkgo@latest".
golangci-lint   See https://golangci-lint.run/usage/install/#local-installation.
vboxheadless    run "VBoxHeadless --help" to diagnose why vboxheadless failed to execute
cfssl           See https://github.com/cloudflare/cfssl#installation.
cfssljson       See https://github.com/cloudflare/cfssl#installation.

See https://docs.cilium.io/en/latest/contributing/development/dev_setup/.
root@1a0f5e8517c5:/go/src/github.com/cilium/cilium# 

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 23, 2022
@sayboras sayboras added the release-note/misc This PR makes changes that have no direct user impact. label Dec 23, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 23, 2022
@sayboras sayboras force-pushed the tam/dev-container branch 2 times, most recently from a3da453 to 51a9170 Compare December 23, 2022 08:54
@sayboras sayboras marked this pull request as ready for review December 23, 2022 08:55
@sayboras sayboras requested review from a team as code owners December 23, 2022 08:55
@sayboras sayboras force-pushed the tam/dev-container branch 2 times, most recently from c0fcf77 to a70aa62 Compare December 23, 2022 09:47
@sayboras sayboras requested a review from a team as a code owner December 23, 2022 09:47
@sayboras sayboras requested review from a team as code owners December 23, 2022 10:00
@sayboras sayboras temporarily deployed to release-base-images December 23, 2022 10:00 — with GitHub Actions Inactive
@sayboras sayboras changed the title dev: Add devcontainer configuration contrib: Add devcontainer configuration Dec 23, 2022
@sayboras sayboras temporarily deployed to release-base-images December 23, 2022 10:31 — with GitHub Actions Inactive
@sayboras sayboras temporarily deployed to release-base-images December 23, 2022 10:44 — with GitHub Actions Inactive
@sayboras
Copy link
Member Author

The changes here are mainly related to local development, full CI is not required.

@sayboras sayboras temporarily deployed to release-base-images December 28, 2022 03:34 — with GitHub Actions Inactive
@sayboras sayboras temporarily deployed to release-base-images December 28, 2022 03:43 — with GitHub Actions Inactive
@sayboras sayboras temporarily deployed to release-base-images January 23, 2023 02:48 — with GitHub Actions Inactive
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Just one nit, otherwise LGTM thanks!

Documentation/contributing/development/dev_setup.rst Outdated Show resolved Hide resolved
@sayboras sayboras temporarily deployed to release-base-images January 24, 2023 13:15 — with GitHub Actions Inactive
@joestringer joestringer added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jan 24, 2023
This is to provide the quick way for new contributors to bootstrap
development environment with [devcontainer], which is widely used in
vscode as well as github [codespaces]. The docker image is completely
re-used from cilium builder image.

Minimum set of features (e.g. docker-in-docker) is enabled by default.
While enabling more features is handy, it will increase the bootstrap
time. Hence, only docker is enabled as required by Makefile targets (e.g
helm-docs).

[devcontainer]: https://code.visualstudio.com/docs/devcontainers/containers
[codespaces]: https://docs.github.com/en/codespaces/setting-up-your-project-for-codespaces/introduction-to-dev-containers

Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras sayboras temporarily deployed to release-base-images January 25, 2023 11:23 — with GitHub Actions Inactive
@sayboras
Copy link
Member Author

Couple of forced push to rebase and update {builder,runtime} images.

@sayboras
Copy link
Member Author

sayboras commented Jan 25, 2023

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks N/S loadbalancing Tests with XDP, vxlan tunnel, SNAT and Random

Failure Output

FAIL: Can not connect to service "tftp://[fd04::11]:31231/hello" from outside cluster (2/10)

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

@sayboras sayboras added the sig/contributing Impacts contribution workflow, guidelines, and tools. label Jan 25, 2023
@sayboras
Copy link
Member Author

Travis failure is due to the below issue, which will be fixed in #23329

--- FAIL: TestGetIdentity (0.61s)
                                 
    --- FAIL: TestGetIdentity/Duplicated_identity (0.03s)

@sayboras
Copy link
Member Author

Reviews are in, all jobs are passed except:

Mark this ready to merge so that this PR will show up for tophat.

@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 25, 2023
@borkmann borkmann merged commit fef5625 into cilium:master Jan 26, 2023
@sayboras sayboras deleted the tam/dev-container branch January 26, 2023 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/contributing Impacts contribution workflow, guidelines, and tools.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants