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.

Sign up for GitHub

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

Jump to bottom

[containerd]Fixing container commands when mode is local and state is disabled #24

Closed
wants to merge 3 commits into from
Closed

[containerd]Fixing container commands when mode is local and state is disabled #24

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8bf8a49
[containerd]Fixing container commands when mode is local and state is…
dgsudharsan Feb 8, 2022
d5df356
Removing prints
dgsudharsan Feb 14, 2022
f08b9f2
Removing feature check from container stop as the flow is feature dis…
dgsudharsan Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 44 additions & 21 deletions src/sonic-ctrmgrd/ctrmgr/container
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ CONTAINER_ID = "container_id"
REMOTE_STATE = "remote_state"
VERSION = "container_version"
SYSTEM_STATE = "system_state"
STATE = "state"

KUBE_LABEL_TABLE = "KUBE_LABELS"
KUBE_LABEL_SET_KEY = "SET"
Expand All @@ -38,6 +39,9 @@ SONIC_CTR_CONFIG_PEND_SECS = "revert_to_local_on_wait_seconds"
DEFAULT_PEND_SECS = ( 5 * 60 )
WAIT_POLL_SECS = 2

SUCCESS = 0
FAILURE = -1

remote_ctr_enabled = False

def debug_msg(m):
Expand Down Expand Up @@ -87,10 +91,10 @@ def read_data(is_config, feature, fields):

def read_config(feature):
""" Read requried feature config """
set_owner, no_fallback = read_data(True, feature,
[(SET_OWNER, "local"), (NO_FALLBACK, False)])
set_owner, no_fallback, state = read_data(True, feature,
[(SET_OWNER, "local"), (NO_FALLBACK, False), (STATE, "disabled")])

return (set_owner, not no_fallback)
return (set_owner, not no_fallback, state)


def read_state(feature):
Expand All @@ -107,12 +111,12 @@ def docker_action(action, feature, **kwargs):
container = client.containers.get(feature)
getattr(container, action)(**kwargs)
syslog.syslog(syslog.LOG_INFO, "docker cmd: {} for {}".format(action, feature))
return 0
return SUCCESS

except (docker.errors.NotFound, docker.errors.APIError) as err:
syslog.syslog(syslog.LOG_ERR, "docker cmd: {} for {} failed with {}".
format(action, feature, str(err)))
return -1
return FAILURE


def set_label(feature, create):
Expand Down Expand Up @@ -186,7 +190,7 @@ def container_start(feature, **kwargs):

init()

set_owner, fallback = read_config(feature)
set_owner, fallback, _ = read_config(feature)
_, remote_state, _ = read_state(feature)

debug_msg("{}: set_owner:{} fallback:{} remote_state:{}".format(
Expand Down Expand Up @@ -244,20 +248,25 @@ def container_stop(feature, **kwargs):
debug_msg("BEGIN")

init()

set_owner, _ = read_config(feature)
ret = SUCCESS
set_owner, _ , state = read_config(feature)
current_owner, remote_state, _ = read_state(feature)
docker_id = container_id(feature)
remove_label = (remote_state != "pending") or (set_owner == "local")

debug_msg("{}: set_owner:{} current_owner:{} remote_state:{} docker_id:{}".format(
feature, set_owner, current_owner, remote_state, docker_id))
debug_msg("{}: set_owner:{} current_owner:{} remote_state:{} docker_id:{} state:{}".format(
feature, set_owner, current_owner, remote_state, docker_id, state))

if remove_label:
set_label(feature, False)

if set_owner == "local":
if state not in ["enabled", "always_enabled"]:
debug_msg("{} is not enabled".format(feature))
return FAILURE

if docker_id:
docker_action("stop", docker_id, **kwargs)
ret = docker_action("stop", docker_id, **kwargs)
else:
syslog.syslog(
syslog.LOG_ERR if current_owner != "none" else syslog.LOG_INFO,
Expand Down Expand Up @@ -286,6 +295,7 @@ def container_stop(feature, **kwargs):
update_data(feature, data)

debug_msg("END")
return ret


def container_kill(feature, **kwargs):
Expand All @@ -301,19 +311,27 @@ def container_kill(feature, **kwargs):

init()

set_owner, _ = read_config(feature)
ret = SUCCESS
set_owner, _ , state = read_config(feature)
current_owner, remote_state, _ = read_state(feature)
docker_id = container_id(feature)
remove_label = (set_owner != "local") or (current_owner != "local")

debug_msg("{}: set_owner:{} current_owner:{} remote_state:{} docker_id:{}".format(
feature, set_owner, current_owner, remote_state, docker_id))
print("{}: set_owner:{} current_owner:{} remote_state:{} docker_id:{} state:{}".format(
dgsudharsan marked this conversation as resolved.
Show resolved Hide resolved
feature, set_owner, current_owner, remote_state, docker_id, state))

print(remove_label)
dgsudharsan marked this conversation as resolved.
Show resolved Hide resolved
if remove_label:
set_label(feature, False)

if set_owner == "local":
print("returning failure")
dgsudharsan marked this conversation as resolved.
Show resolved Hide resolved
if state not in ["enabled", "always_enabled"]:
debug_msg("{} is not enabled".format(feature))
return FAILURE

if docker_id:
docker_action("kill", docker_id, **kwargs)
ret = docker_action("kill", docker_id, **kwargs)

else:
syslog.syslog(
Expand All @@ -322,6 +340,7 @@ def container_kill(feature, **kwargs):


debug_msg("END")
return ret


def container_wait(feature, **kwargs):
Expand All @@ -341,10 +360,11 @@ def container_wait(feature, **kwargs):

init()

set_owner, fallback = read_config(feature)
set_owner, fallback, _ = read_config(feature)
current_owner, remote_state, _ = read_state(feature)
docker_id = container_id(feature)
pend_wait_secs = 0
ret = SUCCESS

if not docker_id and fallback:
pend_wait_secs = get_config_data(
Expand Down Expand Up @@ -377,8 +397,9 @@ def container_wait(feature, **kwargs):
format(feature))
else:
debug_msg("END -- transitioning to docker wait")
docker_action("wait", docker_id, **kwargs)
ret = docker_action("wait", docker_id, **kwargs)

return ret

def main():
parser=argparse.ArgumentParser(description="container commands for start/stop/wait/kill/id")
Expand All @@ -389,24 +410,26 @@ def main():
args = parser.parse_args()
kwargs = {}

ret = 0
if args.action == "start":
container_start(args.name, **kwargs)
ret = container_start(args.name, **kwargs)

elif args.action == "stop":
if args.timeout is not None:
kwargs['timeout'] = args.timeout
container_stop(args.name, **kwargs)
ret = container_stop(args.name, **kwargs)

elif args.action == "kill":
container_kill(args.name, **kwargs)
ret = container_kill(args.name, **kwargs)

elif args.action == "wait":
container_wait(args.name, **kwargs)
ret = container_wait(args.name, **kwargs)

elif args.action == "id":
id = container_id(args.name, **kwargs)
print(id)

return ret

if __name__ == "__main__":
main()
89 changes: 87 additions & 2 deletions src/sonic-ctrmgrd/tests/container_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@
common_test.CONFIG_DB_NO: {
common_test.FEATURE_TABLE: {
"snmp": {
"set_owner": "local"
"set_owner": "local",
"state": "enabled"
}
}
},
Expand Down Expand Up @@ -254,6 +255,28 @@
}


# container_stop test cases
# test case 0 -- container stop local for disabled container
#
invalid_stop_test_data = {
0: {
common_test.DESCR: "container stop for local disabled container",
common_test.PRE: {
common_test.CONFIG_DB_NO: {
common_test.FEATURE_TABLE: {
"sflow": {
"set_owner": "local"
}
}
}
},
common_test.POST: {
},
common_test.ACTIONS: {
}
}
}

# container_kill test cases
# test case 0 -- container kill local
# -- no change in state-db
Expand All @@ -269,7 +292,8 @@
common_test.CONFIG_DB_NO: {
common_test.FEATURE_TABLE: {
"snmp": {
"set_owner": "local"
"set_owner": "local",
"state": "enabled"
}
}
},
Expand Down Expand Up @@ -348,6 +372,30 @@
}
}

# container_kill test cases
# test case 0 -- container kill local disabled container
# -- no change in state-db
# -- no label update
#
invalid_kill_test_data = {
0: {
common_test.DESCR: "container kill for local disabled container",
common_test.PRE: {
common_test.CONFIG_DB_NO: {
common_test.FEATURE_TABLE: {
"sflow": {
"set_owner": "local"
}
}
}
},
common_test.POST: {
},
common_test.ACTIONS: {
}
}
}


# container_wait test cases
# test case 0 -- container wait local
Expand Down Expand Up @@ -480,6 +528,25 @@ def test_stop_ct(self, mock_docker, mock_table, mock_conn):
assert ret == 0


@patch("container.swsscommon.DBConnector")
@patch("container.swsscommon.Table")
@patch("container.docker.from_env")
def test_invalid_stop_ct(self, mock_docker, mock_table, mock_conn):
self.init()
common_test.set_mock(mock_table, mock_conn, mock_docker)

for (i, ct_data) in invalid_stop_test_data.items():
common_test.do_start_test("container_test:container_stop", i, ct_data)

ret = container.container_stop("sflow")
assert ret != 0

ret = common_test.check_tables_returned()
assert ret == 0

ret = common_test.check_mock_containers()
assert ret == 0

@patch("container.swsscommon.DBConnector")
@patch("container.swsscommon.Table")
@patch("container.docker.from_env")
Expand All @@ -498,6 +565,24 @@ def test_kill(self, mock_docker, mock_table, mock_conn):
ret = common_test.check_mock_containers()
assert ret == 0

@patch("container.swsscommon.DBConnector")
@patch("container.swsscommon.Table")
@patch("container.docker.from_env")
def test_invalid_kill(self, mock_docker, mock_table, mock_conn):
self.init()
common_test.set_mock(mock_table, mock_conn, mock_docker)

for (i, ct_data) in invalid_kill_test_data.items():
common_test.do_start_test("container_test:container_kill", i, ct_data)

ret = container.container_kill("sflow")

assert ret != 0
ret = common_test.check_tables_returned()
assert ret == 0

ret = common_test.check_mock_containers()
assert ret == 0

@patch("container.swsscommon.DBConnector")
@patch("container.swsscommon.Table")
Expand Down