Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Makefile: Build agent with tag seccomp to support seccomp #353

Merged
merged 1 commit into from
Sep 11, 2018
Merged

Makefile: Build agent with tag seccomp to support seccomp #353

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ INIT := no
UNIT_DIR := /usr/lib/systemd/system

GENERATED_FILES :=
# Define if agent will be built by seccomp tag
SECCOMP := no

ifeq ($(INIT),no)
# Unit file to start kata agent in systemd systems
Expand All @@ -31,6 +33,9 @@ COMMIT_NO_SHORT := $(shell git rev-parse --short HEAD 2> /dev/null || true)
COMMIT := $(if $(shell git status --porcelain --untracked-files=no),${COMMIT_NO}-dirty,${COMMIT_NO})
VERSION_COMMIT := $(if $(COMMIT),$(VERSION)-$(COMMIT),$(VERSION))
ARCH := $(shell go env GOARCH)
ifeq ($(SECCOMP),yes)
BUILDTAGS := seccomp
endif
Copy link

Choose a reason for hiding this comment

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

The same way we have INIT := no by default, please make sure we have SECCOMP := no too.
This way, we can merge this PR, but leaving the default way of building the agent the way it is.

Copy link
Contributor Author

@nitkon nitkon Sep 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@jodh-intel @sboeuf : Sorry, I didn't understand the comment completely.

  1. When SECCOMP=yes will be passed when making a local rootfs image, If we use SECCOMP := no in this Makefile, just like INIT, it shall override its value, thus having no effect of environment variable passed.
  2. We can set SECCOMP to no only if it's not set, but then still agent would be built the same way go build -tags "" -o kata-agent -ldflags "-X main.version=1.2.0-5765593309eab087598c093ff226500af158df9c-dirty"
ifeq ($(SECCOMP),)
SECCOMP := no
endif

Probably I am missing something.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I'm a bit confused now too as your code will work as we want currently.

I think @sboeuf might simply be suggesting that for consistency with the INIT option that you supplement your change with an explicit:

# Set to 'yes' to build agent with seccomp support
SECCOMP := no

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jodh-intel : If I set SECCOMP := no in the Makefile like INIT it would over-ride what was passed by the user as an environment variable when building the rootfs.

Probably this is what he means? To set it to "no" if not set by the user?
https://github.com/kata-containers/osbuilder/blob/93ad0491ef6d3d0b9d20956071b6b39d0a787b4d/rootfs-builder/rootfs.sh#L16

Copy link
Contributor

Choose a reason for hiding this comment

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

@nitkon: variables passed from command line have priority over definitions inside Makefiles: https://www.gnu.org/software/make/manual/make.html#Override-Directive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marcov : Oh I was thinking the other way round. Now it makes sense. Update my PR. :-D

Copy link
Contributor

Choose a reason for hiding this comment

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

The only time this won't work is if the user wants to enable seccomp and tries to flag that by setting an actual environment variable:

$ export SECCOMP="yes"
$ make  # SECCOMP will still be "no" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jodh-intel: Yea, I had tried it the same way as you mentioned and so thought it to be the other way round. :-)


# args for building agent image
BUILDARGS := $(if $(http_proxy), --build-arg http_proxy=$(http_proxy))
Expand All @@ -40,7 +45,7 @@ AGENT_IMAGE := katacontainers/agent-dev
AGENT_TAG := $(if $(COMMIT_NO_SHORT),$(COMMIT_NO_SHORT),dev)

$(TARGET): $(GENERATED_FILES) $(SOURCES) $(VERSION_FILE)
go build -o $@ -ldflags "-X main.version=$(VERSION_COMMIT)"
go build -tags "$(BUILDTAGS)" -o $@ -ldflags "-X main.version=$(VERSION_COMMIT)"

install:
install -D $(TARGET) $(DESTDIR)$(BINDIR)/$(TARGET)
Expand Down