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

Set proper hostname on containers startup #2806

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions files/build_templates/docker_image_ctl.j2
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ function getMountPoint()
echo $1 | python -c "import sys, json, os; mnts = [x for x in json.load(sys.stdin)[0]['Mounts'] if x['Destination'] == '/usr/share/sonic/hwsku']; print '' if len(mnts) == 0 else os.path.basename(mnts[0]['Source'])" 2>/dev/null
}

function updateHostName()
{
HOSTNAME=`hostname`

echo "Set hostname in {{docker_container_name}} container"
docker exec -i {{docker_container_name}} bash -c "hostname $HOSTNAME"
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 23, 2019

Choose a reason for hiding this comment

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

docker exec -i {{docker_container_name}} bash -c "hostname $HOSTNAME" [](start = 4, length = 69)

It is possible that the early started processes see the original hostname.

Copy link
Author

Choose a reason for hiding this comment

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

i've checked this using 'dhcp_relay' testcase which is required the hostname to pre-set to a known state and it's working. Anyway, it's better than the existing state as we'll keep hostname to be in sync w/ base system.
In addition - as for my understanding, there is no way to set some specific hostname while starting the container. the only way to set a proper hostname for all processes - remove the previous instance of a container each time.. but this could bring timing issue.

Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 23, 2019

Choose a reason for hiding this comment

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

This is the comparison between your PR and #2773. That one has limited scope on only one container, and don't allow early early processes get the original hostname, because the processes order are controlled by start.sh.

This proposed PR is meant to be general, but has this disadvantage. So it is not qualified for a complete solution.

If you require any fix on specific container, you may follow that PR and fix in the same way.

docker exec -i {{docker_container_name}} bash -c "echo \"$HOSTNAME\" > /etc/host"
}

function getBootType()
{
local BOOT_TYPE
Expand Down Expand Up @@ -81,9 +90,8 @@ function postStartAction()
fi
{%- elif docker_container_name == "snmp" %}
docker exec -i database redis-cli -n 6 HSET 'DEVICE_METADATA|localhost' chassis_serial_number $(decode-syseeprom -s)
{%- else %}
: # nothing
{%- endif %}
updateHostName
}

start() {
Expand Down