Skip to content

Use shutil.move instead of os.rename to move config_db.json on place - #66

Merged
saiarcot895 merged 2 commits into
sonic-net:masterfrom
leenr:fix/shutil-move-config_db-instead-of-os-rename
Apr 14, 2026
Merged

Use shutil.move instead of os.rename to move config_db.json on place#66
saiarcot895 merged 2 commits into
sonic-net:masterfrom
leenr:fix/shutil-move-config_db-instead-of-os-rename

Conversation

@leenr

@leenr leenr commented May 22, 2025

Copy link
Copy Markdown
Contributor

What I did
I replaced os.rename call with shutil.move.
Also, I reordered imports to be in alphabetical order (cosmetic change).

Why I did it
After sonic-net/sonic-buildimage#20640, SONiC systems mount /tmp as separate tmpfs mount.

SONiC ZTP scripts currently prepares config_dl.json file in /tmp folder and then tries to rename file to /etc/sonic/config_db.json. Earlier there was no issue with that, as /tmp and /etc/sonic was residing on the same mount, on the same device. Now (sonic-buildimage master, and 202505 branch - sonic-net/sonic-buildimage@fbd9252), os.rename raises a OSError: [Errno 18] Invalid cross-device link: '/tmp/config_dl.json' -> '/etc/sonic/config_db.json' exception.

Such behaviour of os.rename is documented:

The operation may fail on some Unix flavors if src and dst are on different filesystems.

I replaced os.rename call with shutil.move (docs) one, which should either use os.rename if such operation is possible, or fall back into "manual" course of actions: first copy the file onto new place, then remove the source file.

I suspect this change will also solve #54, but I'm not entirely sure that the cause was the similar one there.

@linux-foundation-easycla

linux-foundation-easycla Bot commented May 22, 2025

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: leenr / name: Vladimir Solomatin (fd57a8c)
  • ✅ login: saiarcot895 / name: Saikrishna Arcot (4f5a94f)

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@leenr

leenr commented Jun 22, 2025

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 66 in repo sonic-net/sonic-ztp

@leenr

leenr commented Jun 22, 2025

Copy link
Copy Markdown
Contributor Author

/azpw run

@mssonicbld

Copy link
Copy Markdown
Collaborator

/AzurePipelines run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@bebeisdog

Copy link
Copy Markdown

I also encountered the same issue. I think this solution is good, but I'm not sure why it hasn't been merged yet. You also explained the root cause

@anders-nexthop

Copy link
Copy Markdown

@wsycqyz

wsycqyz commented Apr 14, 2026

Copy link
Copy Markdown

close pr and reopen pr to trigger pr test.

@wsycqyz wsycqyz closed this Apr 14, 2026
@wsycqyz wsycqyz reopened this Apr 14, 2026
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@saiarcot895
saiarcot895 merged commit c84b50a into sonic-net:master Apr 14, 2026
4 of 5 checks passed
@rajann

rajann commented Apr 14, 2026

Copy link
Copy Markdown

Could this be double committed to 202511 branch?

@mssonicbld

Copy link
Copy Markdown
Collaborator

Cherry-pick PR to 202511: #76

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.

8 participants