Skip to content

Third Party Container Management Support in Sonic App Extension#14917

Merged
Praveen-Brcm merged 1 commit intosonic-net:masterfrom
sg893052:TPCM_MAIN
May 15, 2024
Merged

Third Party Container Management Support in Sonic App Extension#14917
Praveen-Brcm merged 1 commit intosonic-net:masterfrom
sg893052:TPCM_MAIN

Conversation

@sg893052
Copy link
Copy Markdown
Contributor

@sg893052 sg893052 commented May 2, 2023

Why I did it

To support Third Party Container Management in Sonic App Extension Framework

Work item tracking
  • Microsoft ADO (number only):

How I did it

The main changes for this feature are in sonic-utilities repo.
Here in this sonic-buildimage repo, this change is to prepare the directory (subdirectory "manifests" under /var/lib/sonic-package-manager) and the default manifest at build time without the need to create it in runtime every time

How to verify it

Execute commands specified in sonic-utilities repo PR for TPCM.

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

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

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)

@jeff-yin
Copy link
Copy Markdown
Collaborator

jeff-yin commented May 9, 2023

During the presentation today, there was a chat message requesting review sign-ups.

@prvattem from Dell can review. @zhangyanzhao can you help assign?

@sg893052
Copy link
Copy Markdown
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Copy Markdown
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@adyeung
Copy link
Copy Markdown
Collaborator

adyeung commented Jan 12, 2024

@prvattem @venkatmahalingam @stepanblyschak pls review the Code PR

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where is target_machine consumed in default_manifest.j2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

target_machine is not required in default_manifest.j2
Hence removed it now.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why install new package?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If required for sonic-utilities then specify in setup.py - https://github.com/sonic-net/sonic-utilities/blob/master/setup.py#L230

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see paramiko is already present in setup.py.
Hence removed it from here now.

Copy link
Copy Markdown
Collaborator

@stepanblyschak stepanblyschak Jan 20, 2024

Choose a reason for hiding this comment

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

This one will be listed by this command, although not created by the user and can be deleted by the user.

admin@r-moose-01:~$ sudo spm manifests list
Custom Local Manifest files:
- default_manifest

Is that expected?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The default_manifest seems to be duplicated in sonic-utilities PR, why need in both places?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tab vs space inconsistency

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

False -> false
True -> true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems it does not require to be a Jinja template

@sg893052
Copy link
Copy Markdown
Contributor Author

sg893052 commented Apr 1, 2024

This repo change is not required anymore as the changes are accommodated in sonic-utilities repo.

@sg893052 sg893052 closed this Apr 1, 2024
@sg893052
Copy link
Copy Markdown
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Copy Markdown
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@sg893052
Copy link
Copy Markdown
Contributor Author

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Commenter does not have sufficient privileges for PR 14917 in repo sonic-net/sonic-buildimage

@sg893052
Copy link
Copy Markdown
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Copy Markdown
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@sg893052
Copy link
Copy Markdown
Contributor Author

sg893052 commented May 10, 2024

@adyeung This sonic-buildimage repo change for TPCM support is also required as per the build time requirement change (hence reopened it) and is also approved by the Reviewer.
Waiting for the build checks to pass

@Praveen-Brcm Praveen-Brcm merged commit a480f8d into sonic-net:master May 15, 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.

6 participants