Skip to content

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

Merged
qiluo-msft merged 2 commits intosonic-net:masterfrom
vsenchyshyn:snmp-hostname-fix
Apr 16, 2019
Merged

[snmp] Configure snmp docker hostname from config DB#2773
qiluo-msft merged 2 commits intosonic-net:masterfrom
vsenchyshyn:snmp-hostname-fix

Conversation

@vsenchyshyn
Copy link
Contributor

Signed-off-by: Vitaliy Senchyshyn [email protected]

- 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)

@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

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 <[email protected]>
@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