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

Move k8s script to docker-config-engine #14788

Merged

Conversation

lixiaoyuner
Copy link
Contributor

@lixiaoyuner lixiaoyuner commented Apr 21, 2023

Why I did it

To reduce the container's dependency from host system

Work item tracking
  • Microsoft ADO (number only):
    17852636

How I did it

Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it

Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

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)

Signed-off-by: Yun Li <yunli1@microsoft.com>
ganglyu
ganglyu previously approved these changes Apr 28, 2023
Copy link
Contributor

@ganglyu ganglyu left a comment

Choose a reason for hiding this comment

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

LGTM

@lixiaoyuner lixiaoyuner changed the title Move k8s script to config engine container for telemetry Move k8s script to config engine container May 4, 2023
ganglyu
ganglyu previously approved these changes May 8, 2023
@@ -5,7 +5,7 @@ if [ "${RUNTIME_OWNER}" == "" ]; then
fi

CTR_SCRIPT="/usr/share/sonic/scripts/container_startup.py"
if test -f ${CTR_SCRIPT}
if [ $RUNTIME_OWNER = "kube" ] && test -f ${CTR_SCRIPT}
Copy link
Collaborator

@qiluo-msft qiluo-msft May 9, 2023

Choose a reason for hiding this comment

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

test

I am confused:
If $RUNTIME_OWNER=="kube" and CTR_SCRIPT does not exist, you run the script? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code means if $RUNTIME_OWNER=="kube" and CTR_SCRIPT exists, than I run the script

Copy link
Contributor Author

@lixiaoyuner lixiaoyuner Jun 5, 2023

Choose a reason for hiding this comment

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

Will run the script every time and check inside the script to decide to continue code is here.

rules/config Outdated
@@ -178,7 +178,7 @@ INCLUDE_ROUTER_ADVERTISER ?= y

# INCLUDE_KUBERNETES - if set to y kubernetes packages are installed to be able to
# run as worker node in kubernetes cluster.
INCLUDE_KUBERNETES ?= n
INCLUDE_KUBERNETES ?= y
Copy link
Collaborator

@qiluo-msft qiluo-msft May 9, 2023

Choose a reason for hiding this comment

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

y

This is behavior change. Other changes in this file is refactoring. Could you separate into multiple PRs? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I just enable it for this moment to check PR build tests pass rate. Should disable it as default.

@@ -11,7 +11,7 @@ import datetime
import docker
from swsscommon import swsscommon

CTR_STATE_SCR_PATH = '/usr/share/sonic/scripts/container_startup.py'
CTR_STATE_SCR_PATH = '/etc/sonic/remote_ctr.config.json'
Copy link
Collaborator

@qiluo-msft qiluo-msft May 9, 2023

Choose a reason for hiding this comment

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

remote_ctr.config.json

Original it is a script, and name SCR is script. Why it is now a json file? #Closed

Copy link
Contributor Author

@lixiaoyuner lixiaoyuner May 9, 2023

Choose a reason for hiding this comment

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

The logic here is to check whether k8s feature is included. If the file exists, it means included. So just check file whether exists, don't care the content.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you also want to change CTR_STATE_SCR_PATH -> CTRMGRD_SERVICE_PATH

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, changed.

qiluo-msft
qiluo-msft previously approved these changes Jun 21, 2023
@lguohan lguohan disabled auto-merge July 5, 2023 21:44
@lguohan lguohan merged commit ca29197 into sonic-net:master Jul 5, 2023
lixiaoyuner added a commit to lixiaoyuner/sonic-buildimage that referenced this pull request Jul 7, 2023
Why I did it
To reduce the container's dependency from host system

Work item tracking
Microsoft ADO (number only):
17713469
How I did it
Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it
Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

Signed-off-by: Yun Li <yunli1@microsoft.com>
Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com>
@mssonicbld
Copy link
Collaborator

@lixiaoyuner PR conflicts with 202205 branch

@mssonicbld
Copy link
Collaborator

@lixiaoyuner PR conflicts with 202211 branch

yxieca pushed a commit that referenced this pull request Jul 7, 2023
Why I did it
To reduce the container's dependency from host system

Work item tracking
Microsoft ADO (number only):
17713469
How I did it
Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it
Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

Signed-off-by: Yun Li <yunli1@microsoft.com>
Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com>
lixiaoyuner added a commit to lixiaoyuner/sonic-buildimage that referenced this pull request Jul 10, 2023
Why I did it
To reduce the container's dependency from host system

Work item tracking
Microsoft ADO (number only):
17713469
How I did it
Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it
Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

Signed-off-by: Yun Li <yunli1@microsoft.com>
Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com>
lixiaoyuner added a commit to lixiaoyuner/sonic-buildimage that referenced this pull request Jul 10, 2023
Why I did it
To reduce the container's dependency from host system

Work item tracking
Microsoft ADO (number only):
17713469
How I did it
Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it
Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

Signed-off-by: Yun Li <yunli1@microsoft.com>
Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com>
yxieca pushed a commit that referenced this pull request Jul 10, 2023
Why I did it
To reduce the container's dependency from host system

Work item tracking
Microsoft ADO (number only):
17713469
How I did it
Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it
Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

Signed-off-by: Yun Li <yunli1@microsoft.com>
Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com>
StormLiangMS pushed a commit that referenced this pull request Jul 17, 2023
Why I did it
To reduce the container's dependency from host system

Work item tracking
Microsoft ADO (number only):
17713469
How I did it
Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it
Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

Signed-off-by: Yun Li <yunli1@microsoft.com>
Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com>
@mssonicbld
Copy link
Collaborator

@lixiaoyuner PR conflicts with 202305 branch

sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
Why I did it
To reduce the container's dependency from host system

Work item tracking
Microsoft ADO (number only):
17713469
How I did it
Move the k8s container startup script to config engine container, other than mount it from host.

How to verify it
Check file path(/usr/share/sonic/scripts/container_startup.py) inside config engine container.

Signed-off-by: Yun Li <yunli1@microsoft.com>
Co-authored-by: Qi Luo <qiluo-msft@users.noreply.github.com>
@lixiaoyuner lixiaoyuner deleted the add-k8s-script-to-base-container branch February 7, 2024 06:09
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.

9 participants