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

[dockers] change RPC, DBG dockers version: put RPG, DBG sign in build… #8920

Merged
merged 1 commit into from
Nov 1, 2021
Merged

[dockers] change RPC, DBG dockers version: put RPG, DBG sign in build… #8920

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
8 changes: 8 additions & 0 deletions files/build_templates/manifest.json.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
{%- if version_suffix %}
{#- append build metadata suffix to the version #}
{%- if '+' in version %}
{%- set version = version + '.' + version_suffix %}
{%- else %}
{%- set version = version + '+' + version_suffix %}
{%- endif %}
{%- endif %}
{
"version": "1.0.0",
"package": {
Expand Down
2 changes: 1 addition & 1 deletion platform/barefoot/docker-syncd-bfn-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_BFN_RPC)
endif

$(DOCKER_SYNCD_BFN_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_BFN_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_BFN_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_BFN_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_BFN_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_BFN_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/broadcom/docker-syncd-brcm-dnx-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_BRCM_DNX_RPC)
endif

$(DOCKER_SYNCD_BRCM_DNX_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_BRCM_DNX_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_BRCM_DNX_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_BRCM_DNX_RPC)_PACKAGE_NAME = syncd-dnx
$(DOCKER_SYNCD_BRCM_DNX_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_BRCM_DNX_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/broadcom/docker-syncd-brcm-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_BRCM_RPC)
endif

$(DOCKER_SYNCD_BRCM_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_BRCM_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_BRCM_RPC)_VERSION = 1.0.0+rpc
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

1.0.0+rpc

What is the impact to the build result?
Is there a design of the version string?
If we keep -, seem it is just a parsing issue? #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, there is a desing for version string, we are following https://semver.org/. This was a flaw of initial implementation. I chose to put rpc with a dash, which in semver means that this is a pre-release version, which it is not. It causes an extension installation failure that requires syncd>=1.0.0 but according to semver rules 1.0.0-rpc is less then 1.0.0. With this change I put rpc sign as a build metadata part which solves an issue because 1.0.0+rpc satisfies the constraint >=1.0.0.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Trying to learn from you.
How semver is accepted by Debian?
You mentioned that

  1. 1.0.0-rpc is less then 1.0.0
  2. 1.0.0+rpc satisfies the constraint >=1.0.0

Are they just by chance, or they are defined by semver rules and implemented by Debian or apt system?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Debian does not follow semver. They are defined by semver rules and there are online tools to check https://semvercompare.azurewebsites.net/?version=1.0.0&version=1.0.0-rpc&version=1.0.0%252Brpc.

Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Why 1.0.0-rpc < 1.0.0 <= 1.0.0+rpc ? Is it just string comparison?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here is a section defining this - https://semver.org/#spec-item-11.

$(DOCKER_SYNCD_BRCM_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_BRCM_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_BRCM_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/cavium/docker-syncd-cavm-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_CAVM_RPC)
endif

$(DOCKER_SYNCD_CAVM_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_CAVM_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_CAVM_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_CAVM_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_CAVM_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_CAVM_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/centec-arm64/docker-syncd-centec-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_CENTEC_RPC)
endif

$(DOCKER_SYNCD_CENTEC_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_CENTEC_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_CENTEC_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_CENTEC_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_CENTEC_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_CENTEC_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/centec/docker-syncd-centec-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_CENTEC_RPC)
endif

$(DOCKER_SYNCD_CENTEC_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_CENTEC_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_CENTEC_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_CENTEC_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_CENTEC_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_CENTEC_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/innovium/docker-syncd-invm-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_INVM_RPC)
endif

$(DOCKER_SYNCD_INVM_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_INVM_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_INVM_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_INVM_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_INVM_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_INVM_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/marvell-arm64/docker-syncd-mrvl-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_MRVL_RPC)
endif

$(DOCKER_SYNCD_MRVL_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_MRVL_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_MRVL_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_MRVL_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_MRVL_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_MRVL_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/marvell-armhf/docker-syncd-mrvl-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_MRVL_RPC)
endif

$(DOCKER_SYNCD_MRVL_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_MRVL_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_MRVL_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_MRVL_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_MRVL_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_MRVL_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/marvell/docker-syncd-mrvl-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_MRVL_RPC)
endif

$(DOCKER_SYNCD_MRVL_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_MRVL_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_MRVL_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_MRVL_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_MRVL_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_MRVL_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/mellanox/docker-syncd-mlnx-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_MLNX_RPC)
endif

$(DOCKER_SYNCD_MLNX_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_MLNX_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_MLNX_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_MLNX_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_MLNX_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_MLNX_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
2 changes: 1 addition & 1 deletion platform/nephos/docker-syncd-nephos-rpc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_SYNCD_NEPHOS_RPC)
endif

$(DOCKER_SYNCD_NEPHOS_RPC)_CONTAINER_NAME = syncd
$(DOCKER_SYNCD_NEPHOS_RPC)_VERSION = 1.0.0-rpc
$(DOCKER_SYNCD_NEPHOS_RPC)_VERSION = 1.0.0+rpc
$(DOCKER_SYNCD_NEPHOS_RPC)_PACKAGE_NAME = syncd
$(DOCKER_SYNCD_NEPHOS_RPC)_RUN_OPT += --privileged -t
$(DOCKER_SYNCD_NEPHOS_RPC)_RUN_OPT += -v /host/machine.conf:/etc/machine.conf
Expand Down
3 changes: 2 additions & 1 deletion rules/functions
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ endef
# call:
# generate_manifest some_docker.gz, version_suffix
define generate_manifest
$(eval export version=$($(1).gz_VERSION)$(2))
$(eval export version=$($(1).gz_VERSION))
$(eval export version_suffix=$(2))
$(eval export name=$($(1).gz_CONTAINER_NAME))
$(eval export package_name=$($(1).gz_PACKAGE_NAME))
$(eval export asic_service=$(shell [ -f files/build_templates/per_namespace/$(name).service.j2 ] && echo true || echo false))
Expand Down
2 changes: 1 addition & 1 deletion slave.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ $(addprefix $(TARGET_PATH)/, $(DOCKER_DBG_IMAGES)) : $(TARGET_PATH)/%-$(DBG_IMAG
$(eval export $(subst -,_,$(notdir $($*.gz_PATH)))_dbg_pkgs=$(shell printf "$(subst $(SPACE),\n,$(call expand,$($*.gz_DBG_APT_PACKAGES),RDEPENDS))\n" | awk '!a[$$0]++'))
./build_debug_docker_j2.sh $* $(subst -,_,$(notdir $($*.gz_PATH)))_dbg_debs $(subst -,_,$(notdir $($*.gz_PATH)))_image_dbgs > $($*.gz_PATH)/Dockerfile-dbg.j2
j2 $($*.gz_PATH)/Dockerfile-dbg.j2 > $($*.gz_PATH)/Dockerfile-dbg
$(call generate_manifest,$*,-dbg)
Copy link
Collaborator

@xumia xumia Oct 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Is it will change the docker image name or the package name? If no, please skip my comment.
There is a dependency on the "-dbg", see https://github.com/Azure/sonic-buildimage/blob/3bb248bd679d7e0b4b46490dbcc682577b60a4ed/scripts/versions_manager.py#L539
Is is possible to change to use "_dbg"? And change the file as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does not change docker image name nor package name.

$(call generate_manifest,$*,dbg)
# Prepare docker build info
PACKAGE_URL_PREFIX=$(PACKAGE_URL_PREFIX) \
SONIC_ENFORCE_VERSIONS=$(SONIC_ENFORCE_VERSIONS) \
Expand Down