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

[buildsystem]: Fix error 'chown: missing operand after' #13569

Merged
merged 1 commit into from
Feb 23, 2023

Conversation

nazariig
Copy link
Collaborator

@nazariig nazariig commented Jan 31, 2023

Fixes: #13395

This fix resolves ownership configuration for vcache:

Step 24/40 : RUN pip3 install j2cli
 ---> Running in fcc39df62a98
chown: missing operand after '/sonic/target/vcache/docker-base-bullseye'
Try 'chown --help' for more information.

Originally the issue was introduced here: #13287

Why I did it

  • To fix ownership configuration

How I did it

  • Removed redundant stuff

How to verify it

  • N/A

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

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

Description for the changelog

  • N/A

Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.

  • N/A

Link to config_db schema for YANG module changes

  • N/A

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

      .---.        .-----------
     /     \  __  /    ------
    / /     \(  )/    -----
   //////   ' \/ `   ---
  //// / // :    : ---
 // /   /  /`    '--
//          //..\\
       ====UU====UU====
           '//||\\`
             ''``

@nazariig
Copy link
Collaborator Author

PR contains a fix for: #13412

@liushilongbuaa
Copy link
Contributor

We can't build SONiC locally if we use default value SONIC_BUILD_RETRY_COUNT=0.
Line16 is skipped. We can't do that.
PR validation passed, because we set the value to 3.

@nazariig nazariig changed the title [buildsystem]: Fix endless build loop [buildsystem]: Fix error 'chown: missing operand after' Feb 3, 2023
@liat-grozovik
Copy link
Collaborator

@liushilongbuaa could you please review following your comment?

@@ -28,8 +28,7 @@ else
PKG_CACHE_PATH=/sonic/target/vcache/${IMAGENAME}
fi
PKG_CACHE_FILE_NAME=${PKG_CACHE_PATH}/cache.tgz
$SUDO mkdir -p ${PKG_CACHE_PATH}
$SUDO chown $USER $PKG_CACHE_PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

From the log, it looks like $USER="".
But it needs tests. SUDO can't be removed. Because there will be other error message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@liushilongbuaa originally it was without sudo. Please see the PR description. Looks like sudo is not needed since the cache path is created with the regular user privileges long before executing the script during docker container build (environment doesn't have USER variable defined)

Copy link
Contributor

Choose a reason for hiding this comment

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

Building sonic-slave-* is different from build target/sonic-*.gz.
Some needs sudo, but some din't need.
I added sudo because of error message.

@xumia
Copy link
Collaborator

xumia commented Feb 14, 2023

@nazariig , the original issue #13395, has been fixed by #13412, do we still need this PR?

@nazariig
Copy link
Collaborator Author

@nazariig , the original issue #13395, has been fixed by #13412, do we still need this PR?

@xumia the original issue hasn't been fixed yet. We still can see an errors during a build. I have attached logs to the description.

@liushilongbuaa
Copy link
Contributor

Maybe change $USER -> $(whoami) will work.

@nazariig
Copy link
Collaborator Author

nazariig commented Feb 15, 2023

Maybe change USER−>(whoami) will work.

@liushilongbuaa This is not a good way, since we should aim to utilize the user ARG passed to docker during build process and whoami may give a false result (depends on a build context and whether the user was changed or not in a Dockerfile). What's the motivation of having chown? BTW, i have tested slave build with this fix - no issues observed. Also, the bug was originally introduced here: #13287

FYI:

nazariig@sonic-build-server: sonic-buildimage$ tree -ugh target/vcache/
target/vcache/
|-- [nazariig dev      4.0K]  docker-base-buster
|-- [nazariig dev      4.0K]  sonic-slave-bullseye
|   `-- [nazariig dev       124]  cache.tgz
`-- [nazariig dev      4.0K]  sonic-slave-buster
    `-- [nazariig dev       125]  cache.tgz

3 directories, 2 files

@liushilongbuaa
Copy link
Contributor

liushilongbuaa commented Feb 16, 2023

https://dev.azure.com/mssonic/build/_build/results?buildId=201413&view=logs&j=3dc8fd7e-4368-5a92-293e-d53cefc8c4b3&t=44e6c678-cb87-52d9-8547-bcdbd0ad6ae4
PR 13287 is to fix the above build issue. It works. If you revert it, the above build will have issues.

@xumia
Copy link
Collaborator

xumia commented Feb 23, 2023

LGTM, the user should have the permission to access PKG_CACHE_PATH without sudo, /vcache/ or /sonic/target/vcache.
@liushilongbuaa , could you please double confirm it?

@liat-grozovik liat-grozovik merged commit d15f520 into sonic-net:master Feb 23, 2023
@StormLiangMS
Copy link
Contributor

@liushilongbuaa I see your comments is not fully addressed, still has some concern on this PR?

@liushilongbuaa
Copy link
Contributor

@StormLiangMS , just go a head. My concern is fixed through pipeline settins.

@keboliu
Copy link
Collaborator

keboliu commented Feb 24, 2023

When cherry-pick to 202211, this one should be after #13412

@mssonicbld
Copy link
Collaborator

@nazariig PR conflicts with 202205 branch

@yxieca
Copy link
Contributor

yxieca commented Mar 1, 2023

@nazariig please create separate PR for 202205 branch.

@liuh-80
Copy link
Contributor

liuh-80 commented Mar 1, 2023

@nazariig, because manually cherry-pick conflict, please also create cherry-pick PR for 202012 branch.

@nazariig
Copy link
Collaborator Author

nazariig commented Mar 7, 2023

@yxieca / @liuh-80 will do once #13412 cherry-pick is done

@liuh-80
Copy link
Contributor

liuh-80 commented Mar 7, 2023

@nazariig, the cherry-pick labels been removed from #13412 2 weeks ago. so that PR will not cherry-pick anymore.

@nazariig
Copy link
Collaborator Author

nazariig commented Mar 7, 2023

@liuh-80 so what do you suggest? This one can't go without #13412

@mssonicbld
Copy link
Collaborator

@nazariig PR conflicts with 202211 branch

@yxieca
Copy link
Contributor

yxieca commented Mar 8, 2023

@liuh-80 can you remove request flag from this PR since you decided not to back port #13412

@liuh-80
Copy link
Contributor

liuh-80 commented Mar 8, 2023

Remove request label because #13412 will not backport.

StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Mar 28, 2023
Related work items: sonic-net#276, sonic-net#305, sonic-net#332, sonic-net#338, sonic-net#339, sonic-net#1188, sonic-net#1192, sonic-net#1197, sonic-net#1206, sonic-net#1685, sonic-net#1690, sonic-net#1696, sonic-net#1699, sonic-net#1709, sonic-net#1727, sonic-net#1737, sonic-net#1741, sonic-net#1742, sonic-net#2511, sonic-net#2512, sonic-net#2532, sonic-net#2559, sonic-net#2626, sonic-net#2638, sonic-net#2645, sonic-net#2649, sonic-net#2660, sonic-net#2669, sonic-net#2670, sonic-net#2678, sonic-net#10084, sonic-net#11442, sonic-net#11873, sonic-net#12047, sonic-net#12110, sonic-net#12207, sonic-net#12529, sonic-net#12678, sonic-net#13235, sonic-net#13287, sonic-net#13372, sonic-net#13395, sonic-net#13456, sonic-net#13497, sonic-net#13522, sonic-net#13545, sonic-net#13547, sonic-net#13552, sonic-net#13569, sonic-net#13572, sonic-net#13578, sonic-net#13591, sonic-net#13611, sonic-net#13647, sonic-net#13649, sonic-net#13660, sonic-net#13710, sonic-net#13716, sonic-net#13724, sonic-net#13726, sonic-net#13732, sonic-net#13735, sonic-net#13739, sonic-net#13757, sonic-net#13786, sonic-net#13792, sonic-net#13800, sonic-net#13801, sonic-net#13802, sonic-net#13805, sonic-net#13806, sonic-net#13812, sonic-net#13814, sonic-net#13822, sonic-net#13831, sonic-net#13834, sonic-net#13847, sonic-net#13870, sonic-net#13882, sonic-net#13884, sonic-net#13885, sonic-net#13894, sonic-net#13895, sonic-net#13926, sonic-net#13932, sonic-net#13935, sonic-net#13942, sonic-net#13951, sonic-net#13953, sonic-net#13964
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[buildsystem]: Errors are seen during privilege escalation
10 participants