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

[snmp] Configure snmp docker hostname from config DB #2773

Merged
merged 2 commits into from
Apr 16, 2019

Conversation

vsenchyshyn
Copy link
Contributor

Signed-off-by: Vitaliy Senchyshyn vsenchyshyn@barefootnetworks.com

- What I did
Configure snmp docker hostname with the value received from the config DB in order to fix SNMP CT which failed because the "sonic" sysName was always received.

- How I did it
Configure snmp docker hostname in the same way as this is done in hostname-config.sh

- How to verify it
Run snmp CT and verify it will pass

- Description for the changelog

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

Signed-off-by: Vitaliy Senchyshyn <vsenchyshyn@barefootnetworks.com>
@vsenchyshyn
Copy link
Contributor Author

@lguohan @stcheng Please review

@qiluo-msft
Copy link
Collaborator

Could you clarify what is 'snmp CT'?

HOSTNAME=`sonic-cfggen -d -v DEVICE_METADATA[\'localhost\'][\'hostname\']`

echo $HOSTNAME > /etc/hostname
hostname -F /etc/hostname
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 12, 2019

Choose a reason for hiding this comment

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

why not use hostname $HOSTNAME? #Closed

Copy link
Contributor Author

@vsenchyshyn vsenchyshyn Apr 12, 2019

Choose a reason for hiding this comment

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

why not use hostname $HOSTNAME?

In this case /etc/hostname will contain the old hostname. #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.

@qiluo-msft Or you meant only this line: "hostname -F /etc/hostname"? If yes, this can be changed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. Let's keep as is.


In reply to: 274999450 [](ancestors = 274999450)

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comments

@@ -9,6 +9,15 @@ sonic-cfggen -d -y /etc/sonic/snmp.yml -t /usr/share/sonic/templates/snmpd.conf.
mkdir -p /var/sonic
echo "# Config files managed by sonic-config-engine" > /var/sonic/config_status

CURRENT_HOSTNAME=`hostname`
HOSTNAME=`sonic-cfggen -d -v DEVICE_METADATA[\'localhost\'][\'hostname\']`
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 13, 2019

Choose a reason for hiding this comment

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

HOSTNAME [](start = 0, length = 8)

Will you handle command error or empty hostname cases? #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.

HOSTNAME [](start = 0, length = 8)

Will you handle command error or empty hostname cases?

Added validation. Please check.

Signed-off-by: Vitaliy Senchyshyn <vsenchyshyn@barefootnetworks.com>
@vsenchyshyn
Copy link
Contributor Author

@qiluo-msft Could you please merge it?

echo $HOSTNAME > /etc/hostname
hostname -F /etc/hostname

sed -i "/\s$CURRENT_HOSTNAME$/d" /etc/hosts
Copy link
Collaborator

Choose a reason for hiding this comment

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

sed -i "/\s$CURRENT_HOSTNAME$/d" /etc/hosts [](start = 4, length = 43)

This line does not work inside docker container.
ref: http://blog.jonathanargentiero.com/docker-sed-cannot-rename-etcsedl8ysxl-device-or-resource-busy/

Could you double check and fix?

yxieca pushed a commit that referenced this pull request Mar 2, 2020
* [snmp] Configure snmp docker hostname from config DB
* Fixed reviewer comments
qiluo-msft added a commit to qiluo-msft/sonic-buildimage that referenced this pull request Mar 9, 2020
yxieca pushed a commit that referenced this pull request Mar 10, 2020
mihirpat1 pushed a commit to mihirpat1/sonic-buildimage that referenced this pull request Jun 14, 2023
…ic-net#2756)" (sonic-net#2773)

This reverts commit 750e064.
Reverts the PR sonic-net#2756

The fix added breaks the previously added workaround sonic-net#2626. Hence requesting to revert the fix.
Once we find a proper solution for sonic-net#12361 we need to reintegrate this PR
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.

3 participants