Fix manual devcontainer workflow on Linux

[eriknordmark]

This ensures that devcontainer can be built and manually run.

Fixes issue #11 and issue #16

[deitch]

Suggestion

[deitch]

Some suggestions:

1. Can we make this a single command?
2. We shouldn't need network=host, as it by default has bridged internet access.
3. This may make the files owned by root.
4. Give a name to the image?
5. Why does the TARGETOS need to be linux? It doesn't particularly care, but that is what it is by default anyways.

So perhaps:

$ docker build -f .devcontainer/Dockerfile -t eve-api-builder .
$ docker run -v $(pwd):/src -w /src -u $(id -u) eve-api-builder make proto

[eriknordmark]

I'll try those commands.

TARGETOS is blank by default (for some unknown reason) which resulted in a 404 error when download.

[deitch]

Ah so set a default in the dockerfile.

[rouming]

That is a good change. I ran Avi's commands, because the last one calls 'make proto' directly. I suggest the following changes:

1. Make TARGETOS=linux by default:

[roman@finn eve-api]$ git diff .devcontainer/
diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile
index 6bc4a3f4cf06..9ef638328940 100644
--- a/.devcontainer/Dockerfile
+++ b/.devcontainer/Dockerfile
@@ -1,7 +1,7 @@
 # syntax=docker/dockerfile:1.6.0
 FROM alpine:3.18
 
-ARG TARGETOS
+ARG TARGETOS=linux
 ARG TARGETARCH
 ARG PROTOC_GEN_GO_VERSION=1.31.0
 ARG PROTOC_VERSION=23.4

2. Add '--rm' to the 'run' command to delete started container:

docker run --rm -v $(pwd):/src -w /src -u $(id -u) eve-api-builder make proto

Then everything works! Thanks all.

[rouming]

And why can't we include these two commands into the makefile under the 'make proto' target? Then no readme and everyone will be familiar with the procedure by calling 'make proto' from the eve-api folder:

diff --git a/Makefile b/Makefile
index db775d116082..15c37e801d2f 100644
--- a/Makefile
+++ b/Makefile
@@ -1,12 +1,16 @@
+proto:
+       docker build -f .devcontainer/Dockerfile -t eve-api-builder .
+       docker run --rm -v $(PWD):/src -w /src -u $$(id -u) eve-api-builder make gen-proto
+
 proto-diagram:
        protodot -inc /usr/local/include -src ./proto/config/devconfig.proto -output devconfig -generated ./images
        dot ./images/devconfig.dot -Tpng -o ./images/devconfig.png
        dot ./images/devconfig.dot -Tsvg -o ./images/devconfig.svg
        echo generated ./images/devconfig.*
 
-.PHONY: proto-api-%
+.PHONY: proto-api-% proto gen-proto
 
-proto: go python proto-diagram
+gen-proto: go python proto-diagram
        @echo Done building protobuf, you may want to vendor it into your packages, e.g. pkg/pillar.
        @echo See ./go/README.md for more information.

NOTE: 'proto' goal is defined at the beginning of the makefile to make it default, so just 'make' or 'make proto' are equivalents.

[deitch]

Yes, it is, but it is not standard to call a docker wrapper or not to call a docker wrapper. We want the clean, simple way to be the simplest command, i.e. make proto. If you want a container to build it for you, that is an extra layer.

I understand where you are coming from; I used to do all of my Makefiles with calls to docker, and you passed a variable to override it, usually something like: make all BUILD=local or similar. Actually, I think an early eve Makefile had just that. But it gets really messy really quickly (our eve Makefile is awfully complex, especially where we try to build the docker builder).

The simplest and most understandable it:

• make proto - I will build with whatever tools I find, I assume they are here. Whether you have them locally, or call them via a docker container, or have some tool that installs them, or use devcontainers (which I really hope matures soon, although the VS Code and GitHub workspaces implementations are pretty good), I will just take it. Lowest and simplest.
• make proto BUILD=docker (or make proto-docker, etc) - I will do the build by wrapping it with some builder layer, which means I will call the builder layer, and then call the simple make proto inside there.

[eriknordmark]

Does this automated build discussion belong in this manual build PR? How do we move forward?

[deitch]

I think it does. In the end, we are just debating if:

• make proto calls the commands and make proto BUILD=container (or make proto-container) wraps it with docker run; OR
• make proto BUILD=local (or make proto-local) calls the commands and make proto calls the container and wraps it with docker run

I prefer the first, based on our experiences with eve; Roman prefers the second.

[rouming]

I prefer just to call make proto, but how it works under the hood it does not matter, until I don't need to build/install any new tools (devcontainer, vscode, etc).

[deitch]

Well, you already have to install docker, so already some tools.

I know, I understand. The principle still applies: the most basic command - the one that will be executed when all prerequisites are in place, i.e. when you are inside your ready-to-go build environment (a container, local with all tools, devcontainer via CLI or vscode, whatever) - is the simplest command, so that needs to be make proto.

If you are outside such an environment and need to launch it, and want a make target for that (which I understand and agree with completely), then you need a higher-level command, so one of:

• make proto-container
• make proto BUILD=container

It doesn't matter which (pick one), as long it is wrapping the basic command, so that the simplest command (make proto) is run inside your ready-to-run dev environment.

Just pick one, put it in, let's do this.

[rouming]

Each run of this command creates a container and does not delete it on command exit. What if to add '--rm' as I noted in the comment?

[eriknordmark]

Sorry I forgot to push the latest yesterday.

[rouming]

Ack.

[rouming]

Can we merge this?

[deitch]

yes, but I do not have merge rights.

[eriknordmark]

yes, but I do not have merge rights.

Odd. But now you do.