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

Add j2 template for enable pam_limit and limit SSH session #10177

Merged
merged 13 commits into from
Mar 31, 2022

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Mar 8, 2022

Why I did it

When too many user login concurrently and run commands, SONiC may kernel panic on some device which has very limited memory.

How I did it

Add j2 template for setup pam_limit plugin for limit SSH session per-user.

How to verify it

Manually validate the j2 template can generate correct config file.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Add j2 template for setup pam_limit plugin for limit SSH session per-user.

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

@liuh-80 liuh-80 requested a review from qiluo-msft March 8, 2022 08:09
@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 9, 2022

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 9, 2022

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 9, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 marked this pull request as ready for review March 10, 2022 05:48
@liuh-80 liuh-80 requested a review from lguohan as a code owner March 10, 2022 05:48
@liuh-80 liuh-80 force-pushed the dev/liuh/add_ssh_limit_template branch from e72797e to e5be05b Compare March 14, 2022 04:55
@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 18, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -1015,6 +1082,7 @@ class HostConfigDaemon:
radius_global = self.config_db.get_table('RADIUS')
radius_server = self.config_db.get_table('RADIUS_SERVER')
self.aaacfg.load(aaa, tacacs_global, tacacs_server, radius_global, radius_server)
self.pamLimitsCfg.load(self.config_db)
Copy link
Collaborator

@qiluo-msft qiluo-msft Mar 19, 2022

Choose a reason for hiding this comment

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

load

Please be aware there is a comprehensive refactor (with some improvement) PR #10168.

This pamLimitsCfg.load could be move into __init__(). All the code in HostConfigDaemon.load() are relating to subscribed tables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, move the load code to init()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After move code to init, I found hostcfgd continuously write PAM error message to syslog, also 'frontend' process take almost 100% CPU, however the config file rendered correctly, so I will check why this happpen and fix it soon.

Copy link
Contributor Author

@liuh-80 liuh-80 Mar 23, 2022

Choose a reason for hiding this comment

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

Fixed The UT issue and 'frontend' process issue fixed by remove unnecessary command, the pam-auth-update code request user input which cause high CPU utilization in pamd, also will update 201811 PR.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 22, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 22, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 23, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 23, 2022

Some UT failed but not related with this PR, I validate this by following test PR which change nothing: https://github.com/Azure/sonic-buildimage/pull/10329/checks?check_run_id=5653802859

and according to failed UT, the issue seems related with this change in sonic-mgmt:
sonic-net/sonic-mgmt@03cccf7

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 23, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 24, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 28, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 29, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80
Copy link
Contributor Author

liuh-80 commented Mar 31, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 merged commit 271ef69 into sonic-net:master Mar 31, 2022
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.

2 participants