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

03.Version-cache - framework environment settings #12003

Closed

Conversation

Kalimuthu-Velappan
Copy link
Contributor

@Kalimuthu-Velappan Kalimuthu-Velappan commented Sep 8, 2022

It defines and passes the necessary version cache environment variables to the caching framework.
It adds the utils script for shared cache file access.
It also adds the post-cleanup logic for cleaning the unwanted files from the docker/image after the version cache creation.

The following variable controls the version caching feature. By default, it is turned off.
SONIC_VERSION_CACHE_METHOD = [none/cache]
SONIC_VERSION_CACHE_SOURCE = /path/to/local/storage

Why I did it

It provides a framework for caching build files during the docker build.

How I did it

It uses three staging processes to create a sonic docker image.

  • Stage1: Creates the sonic docker image
  • Stage2: Copy the intermediate files from the docker image
  • Stage3: Remove the intermediate file from the docker image and creates the final sonic docker image.

How to verify it

Cache files are created under the version cache path after the build.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@Kalimuthu-Velappan
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adyeung
Copy link
Collaborator

adyeung commented Sep 14, 2022

@xumia @liushilongbuaa submitter has taken the time to split the original code PR to smaller submissions for review, pls help take a look

rules/functions Outdated
Comment on lines 51 to 52
LOG_SAVE = |& $(PROJECT_ROOT)/scripts/process_log.sh $(PROCESS_LOG_OPTION) &>> $(PROJECT_ROOT)/$@.log ; test $${PIPESTATUS[-2]} -eq 0 || { [ $$? -eq 0 ] || pushd $(PROJECT_ROOT) > /dev/null ; ./update_screen.sh -e $@ ; popd > /dev/null ; false ; }
LOG = < /dev/null |& $(PROJECT_ROOT)/scripts/process_log.sh $(PROCESS_LOG_OPTION) &>> $(PROJECT_ROOT)/$@.log ; test $${PIPESTATUS[-2]} -eq 0 || { [ $$? -eq 0 ] || pushd $(PROJECT_ROOT) > /dev/null ; ./update_screen.sh -e $@ ; popd > /dev/null ; false ; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to use the same variable form LOG_SAVE and LOG? Looks like the only difference is the "< /dev/null " in the variable LOG.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

# Decode the cache content into gz format
SRC_VERSION_PATH=files/build/versions
if [[ -e ${CACHE_ENCODE_FILE} ]]; then

Copy link
Collaborator

@xumia xumia Oct 21, 2022

Choose a reason for hiding this comment

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

removing the empty line 47?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 9 to 10
local filename=$(dirname $1)
local timeout=${2:-360}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please fix the tab/space format issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@zhangyanzhao
Copy link
Collaborator

@xumia @liushilongbuaa can you please review the PR? Thanks.

@Kalimuthu-Velappan Kalimuthu-Velappan force-pushed the ENV_SETTINGS branch 3 times, most recently from 1c4c75d to a38b1c6 Compare December 2, 2022 09:19
It defines and passes the necessary version cache environment variables
to the caching framework.

It adds the utils script for shared cache file access.

It also adds the post-cleanup logic for cleaning the unwanted files from
the docker/image after the version cache creation.

The following variable controls the version caching feature. By default, it is turned off.
    SONIC_VERSION_CACHE_METHOD = [none/cache]
    SONIC_VERSION_CACHE_SOURCE = /path/to/local/storage
@@ -558,7 +563,7 @@ $(addprefix $(DEBS_PATH)/, $(SONIC_ONLINE_DEBS)) : $(DEBS_PATH)/% : .platform \
if [ -z '$($*_CACHE_LOADED)' ] ; then

$(foreach deb,$* $($*_DERIVED_DEBS), \
{ curl -L -f -o $(DEBS_PATH)/$(deb) $($(deb)_CURL_OPTIONS) $($(deb)_URL) $(LOG) || { exit 1 ; } } ; )
{ SKIP_BUILD_HOOK=$($*_SKIP_VERSION) curl -L -f -o $(DEBS_PATH)/$(deb) $($(deb)_CURL_OPTIONS) $($(deb)_URL) $(LOG) || { exit 1 ; } } ; )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we need to skip it? Can we keep the build hook?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any particular package that wants to skip versioning selectively can define as below. Eg:
$(BRCM_SAI)_SKIP_VERSION=y

slave.mk Show resolved Hide resolved
slave.mk Show resolved Hide resolved
@xumia
Copy link
Collaborator

xumia commented Dec 7, 2022

@Kalimuthu-Velappan , could you please update the description? Add something in the Why/How/How parts, thanks.

@Kalimuthu-Velappan
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Kalimuthu-Velappan
Copy link
Contributor Author

It is merged as part of
#12005

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants