Skip to content

Add log files into the cache tar for improved debugging#18026

Merged
qiluo-msft merged 6 commits intosonic-net:masterfrom
vivekrnv:cache_log
Mar 23, 2024
Merged

Add log files into the cache tar for improved debugging#18026
qiluo-msft merged 6 commits intosonic-net:masterfrom
vivekrnv:cache_log

Conversation

@vivekrnv
Copy link
Contributor

@vivekrnv vivekrnv commented Feb 3, 2024

Why I did it

Cache currently does not save the log of the deb. But it's useful to have the log to understand any cache related problems.

Work item tracking
  • Microsoft ADO (number only):

How I did it

How to verify it

vkarri@s-build-sonic-01:/sonic-buildimage$ tar -tvf /sonic/sonic_caches/libnl-3-200_3.5.0-1_amd64.deb-adc83b19e793491b1c6ea0f-0ea77d65c49ae3a11dffbca.tgz
-rw-r--r-- vkarri/dip   496230 2024-01-12 19:49 target/debs/bookworm/libnl-3-200_3.5.0-1_amd64.deb.log

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

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

@qiluo-msft
Copy link
Collaborator

@xumia Please help review this PR.

@vivekrnv
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).

@vivekrnv
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).

@k-v1
Copy link
Contributor

k-v1 commented Feb 26, 2024

@vivekrnv

My suggestion is to use something like this:

-       $($(1)_CACHE_USER) tar -C $($(1)_BASE_PATH) -mczvf $($(1)_CACHE_DIR)/$(MOD_CACHE_FILE) $(2) $(addprefix $($(1)_DST_PATH)/,$($(1)_DERIVED_DEBS) $($(1)_EXTRA_DEBS) )  \
+       cp $($(1)_DST_PATH)/$(1).log $($(1)_DST_PATH)/$(1).cached.log
+       md5sum $(2) $(addprefix $($(1)_DST_PATH)/,$($(1)_DERIVED_DEBS) $($(1)_EXTRA_DEBS)) >> $($(1)_DST_PATH)/$(1).cached.log
+       $($(1)_CACHE_USER) tar -C $($(1)_BASE_PATH) -mczvf $($(1)_CACHE_DIR)/$(MOD_CACHE_FILE) $(2) $(addprefix $($(1)_DST_PATH)/,$($(1)_DERIVED_DEBS) $($(1)_EXTRA_DEBS) $(1).cached.log)  \
                1>>$($(1)_DST_PATH)/$(1).log
        sudo chmod 777 $($(1)_CACHE_DIR)/$(MOD_CACHE_FILE)
+       rm -f $($(1)_DST_PATH)/$(1).cached.log

Comparing to your solution:

  1. Dont overwrite original log file. You have two log files: pkg.log and pkg.cached.log
  2. Don't split tar and gzip compression into multiple parts (but you don't log compression itself in this case. I'm not sure is it important or not).
  3. Additional step with calculation md5sum of cached files. It can be useful sometimes.

But need more testing. I have tested this for one docker image and deb package.

@vivekrnv
Copy link
Contributor Author

@vivekrnv

My suggestion is to use something like this:

-       $($(1)_CACHE_USER) tar -C $($(1)_BASE_PATH) -mczvf $($(1)_CACHE_DIR)/$(MOD_CACHE_FILE) $(2) $(addprefix $($(1)_DST_PATH)/,$($(1)_DERIVED_DEBS) $($(1)_EXTRA_DEBS) )  \
+       cp $($(1)_DST_PATH)/$(1).log $($(1)_DST_PATH)/$(1).cached.log
+       md5sum $(2) $(addprefix $($(1)_DST_PATH)/,$($(1)_DERIVED_DEBS) $($(1)_EXTRA_DEBS)) >> $($(1)_DST_PATH)/$(1).cached.log
+       $($(1)_CACHE_USER) tar -C $($(1)_BASE_PATH) -mczvf $($(1)_CACHE_DIR)/$(MOD_CACHE_FILE) $(2) $(addprefix $($(1)_DST_PATH)/,$($(1)_DERIVED_DEBS) $($(1)_EXTRA_DEBS) $(1).cached.log)  \
                1>>$($(1)_DST_PATH)/$(1).log
        sudo chmod 777 $($(1)_CACHE_DIR)/$(MOD_CACHE_FILE)
+       rm -f $($(1)_DST_PATH)/$(1).cached.log

Comparing to your solution:

  1. Dont overwrite original log file. You have two log files: pkg.log and pkg.cached.log
  2. Don't split tar and gzip compression into multiple parts (but you don't log compression itself in this case. I'm not sure is it important or not).
  3. Additional step with calculation md5sum of cached files. It can be useful sometimes.

But need more testing. I have tested this for one docker image and deb package.

This sounds good. Will update (Except md5sum)

@vivekrnv vivekrnv requested review from k-v1 and xumia February 27, 2024 22:17
@dgsudharsan
Copy link
Collaborator

@qiluo-msft Can you please help to review this?

@qiluo-msft
Copy link
Collaborator

@xumia Do you have more concern?

Copy link
Collaborator

@xumia xumia left a comment

Choose a reason for hiding this comment

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

LGTM

@qiluo-msft qiluo-msft merged commit 6324fe1 into sonic-net:master Mar 23, 2024
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.

7 participants