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.

Sign up for GitHub

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

Jump to bottom

Add support to run Windows OVS in container #5052

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Add support to run Windows OVS in container #5052

Show file tree
Hide file tree
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
4 changes: 4 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ CNI_BINARIES_VERSION := $(shell head -n 1 build/images/deps/cni-binaries-version
NANOSERVER_VERSION := $(shell head -n 1 build/images/deps/nanoserver-version)
BUILD_TAG := $(shell build/images/build-tag.sh)
WIN_BUILD_TAG := $(shell echo $(GO_VERSION) $(CNI_BINARIES_VERSION) $(NANOSERVER_VERSION)|md5sum|head -c 10)
WIN_OVS_VERSION := $(shell head -n 1 build/images/deps/ovs-version-windows)
WIN_BUILD_OVS_TAG := $(NANOSERVER_VERSION)-$(WIN_OVS_VERSION)
GIT_HOOKS := $(shell find hack/git_client_side_hooks -type f -print)
DOCKER_NETWORK ?= default
TRIVY_TARGET_IMAGE ?=
Expand All @@ -29,6 +31,7 @@ WIN_BUILD_ARGS := --build-arg GO_VERSION=$(GO_VERSION)
WIN_BUILD_ARGS += --build-arg CNI_BINARIES_VERSION=$(CNI_BINARIES_VERSION)
WIN_BUILD_ARGS += --build-arg NANOSERVER_VERSION=$(NANOSERVER_VERSION)
WIN_BUILD_ARGS += --build-arg WIN_BUILD_TAG=$(WIN_BUILD_TAG)
WIN_BUILD_ARGS += --build-arg WIN_BUILD_OVS_TAG=$(WIN_BUILD_OVS_TAG)

.PHONY: all
all: build
Expand Down Expand Up @@ -383,6 +386,7 @@ manifest:
$(CURDIR)/hack/generate-standard-manifests.sh --mode dev --out build/yamls
$(CURDIR)/hack/generate-manifest-windows.sh --mode dev > build/yamls/antrea-windows.yml
$(CURDIR)/hack/generate-manifest-windows.sh --mode dev --containerd > build/yamls/antrea-windows-containerd.yml
$(CURDIR)/hack/generate-manifest-windows.sh --mode dev --containerd --include-ovs > build/yamls/antrea-windows-containerd-with-ovs.yml
$(CURDIR)/hack/generate-manifest-flow-aggregator.sh --mode dev > build/yamls/flow-aggregator.yml

.PHONY: manifest-scale
Expand Down
8 changes: 8 additions & 0 deletions build/images/Dockerfile.build.windows
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

ARG WIN_BUILD_TAG
ARG NANOSERVER_VERSION
ARG WIN_BUILD_OVS_TAG

FROM antrea/base-windows:${WIN_BUILD_TAG} as antrea-build-windows

Expand All @@ -27,6 +28,8 @@ COPY . /antrea

RUN sh -c 'make windows-bin'

FROM antrea/windows-ovs:${WIN_BUILD_OVS_TAG} as windows-ovs
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this image is including files from ovs image layer, we should make sure whenever we have new antrea-ovs image, we shld generate new antrea-windows image .


FROM mcr.microsoft.com/powershell:lts-nanoserver-${NANOSERVER_VERSION}
SHELL ["pwsh", "-NoLogo", "-Command", "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'SilentlyContinue';"]

Expand All @@ -44,3 +47,8 @@ COPY --from=antrea-build-windows /antrea/bin/antrea-cni.exe /k/antrea/cni/antre

RUN mkdir C:\k\antrea\utils
COPY --from=antrea-build-windows /wins/wins.exe /k/antrea/utils/wins.exe

COPY --from=windows-ovs /Windows/System32/vcruntime140.dll /Windows/System32/
Copy link
Member

Choose a reason for hiding this comment

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

have we checked if this file is also the only dependency to run ovsdb-server and ovs-vswitchd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have verified that this file is not the only dependency, we need other two following dll files as well to run ovs commands.

Copy link
Contributor

Choose a reason for hiding this comment

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

So do you mean we can run ovs-vswitchd and ovsdb-server inside nano-server as long as vcruntime140 and the two ssl dll files exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Contributor

Choose a reason for hiding this comment

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

@Atish-iaf ,
there are multiple vc files in system32 folder added after vc installation, can you check if other vc files were already present !

-a---- 5/10/2023 7:01 AM 414104 vcamp140.dll
-a---- 5/10/2023 7:01 AM 346008 vccorlib140.dll
-a---- 5/10/2023 7:01 AM 191864 vcomp140.dll
-a---- 5/10/2023 7:01 AM 109440 vcruntime140.dll
-a---- 5/10/2023 7:01 AM 49560 vcruntime140_1.dll

Copy link
Member

Choose a reason for hiding this comment

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

We can continue the current approach first, then have a separate PR to unify the image and validate everything works fine. I feel we'd better get it done in this release to avoid the inconsistency that user needs to pull an extra image in v1.13 only.

Copy link
Contributor

@wenyingd wenyingd Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If we can run ovsdb-server and ovs-vswitchd with container image antrea-windows, it would be best to use a single image in antrea manifest. A concern is if ovsdb-server and ovs-vswitchd can run correctly in nano-server base or not, not only with your mentioned "--help" command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tnqn we can merge this PR first as this is ready for merge.
To unify image we can continue on PR #5305 and try to get everything validated before release.
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we can run ovsdb-server and ovs-vswitchd with container image antrea-windows, it would be best to use a single image in antrea manifest. A concern is if ovsdb-server and ovs-vswitchd can run correctly in nano-server base or not, not only with your mentioned "--help" command.

@wenyingd In PR #5305 containerd-e2e test passed, so i think it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wenyingd @tnqn Do you have more comments? Once this PR is merged, I think we can proceed with the image unification update in #5305.

COPY --from=windows-ovs /Windows/System32/libeay32.dll /Windows/System32/
COPY --from=windows-ovs /Windows/System32/ssleay32.dll /Windows/System32/
COPY --from=windows-ovs /openvswitch/usr/bin /k/antrea/bin/
Loading