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 hostname in dhcp_relay container #868

Closed
wants to merge 1 commit into from
Closed

Set hostname in dhcp_relay container #868

wants to merge 1 commit into from

Conversation

pyadvichuk
Copy link

@pyadvichuk pyadvichuk commented Apr 12, 2019

Summary:

dhcp_relay testcase expects to see {{ inventory_hostname }} as a part of Option82::Agent Circuit ID but, inside docker container the hostname is sonic. That's why the testcase failed.

Type of change

  • Bug fix
  • [] Testbed and Framework(new/improvement)
  • [] Test case(new/improvement)

Approach

How did you do it?

execute remote shell to set hostname in the container

How did you verify/test it?

run dhcp_relay testacase, checked status

Any platform specific information?

no

@lguohan, @yxieca: please review and merge this.

@lguohan lguohan requested a review from jleveque April 17, 2019 18:07
@jleveque
Copy link
Contributor

@pyadvichuk: I have not experienced the behavior you're describing. Can you please post some output showing this behavior?

@pyadvichuk
Copy link
Author

@pyadvichuk: I have not experienced the behavior you're describing. Can you please post some output showing this behavior?

actually this comes from RFC 3046, section 2.0 - Relay Agent Information Option
but formally we can get the explanation directly from ptftests/dhcp_relay_test.py

>>> ptftests/dhcp_relay_test.py

        # option82 is a byte string created by the relay agent. It contains the circuit_id and remote_id fields.
        # circuit_id is stored as suboption 1 of option 82.
        # It consists of the following:
        #  Byte 0: Suboption number, always set to 1
        #  Byte 1: Length of suboption data in bytes
        #  Bytes 2+: Suboption data
        # Our circuit_id string is of the form "hostname:portname"
        circuit_id_string = self.hostname + ":" + self.client_iface_alias
        self.option82 = struct.pack('BB', 1, len(circuit_id_string))
        self.option82 += circuit_id_string

actually, it correspond to the implementation details of Linux dhcrelay program.
and, of courese, PTF expects to see such string in the relayed packet.

>>> ptftests/dhcp_relay_test.py
.......
def create_dhcp_request_relayed_packet(self):
......
        bootp /= scapy.DHCP(options=[('message-type', 'request'),
                    ('requested_addr', self.client_ip),
                    ('server_id', self.server_ip),
                    ('relay_agent_Information', self.option82),          <------------
                    ('end')])

so.. that's why it is required to have correct hostname inside dhcp_relay container - bc PTF will perform exactly matching. But the hostname defined for device in config_db.json::DEVICE_METADATA::localhost::hostname is applying only in base system, all the containers still have default hostname - sonic.

Probably, it is required to start the discussion regarding setting correct hostname for containers but i have no any information about dev plans and even can't imagine the scope of changes to implement this.

So, from my understanding proposed fix is the simplest and fastest way.

@jleveque
Copy link
Contributor

jleveque commented Apr 17, 2019

I understand how our Option 82 circuit_id is supposed to be formatted. I developed the patch for dhcrelay to support this format.

I was asking for logs and/or output from one of your affected devices, because we do not see this issue. When I run hostname in any Docker container, I get the proper hostname, and our relayed DHCP packets have the proper circuit_id. Our tests do not fail.

Can you please provide more details/output? What version of SONiC are you running?

@pyadvichuk
Copy link
Author

I understand how our Option 82 circuit_id is supposed to be formatted. I developed the patch for this.

I was asking for logs and/or output from one of your affected devices, because we do not see this issue. When I run hostname in any Docker container, I get the proper hostname, and our relayed DHCP packets have the proper circuit_id.

Can you please provide more details/output? What version of SONiC are you running?

It's built from master by jenkins job Mar 30, 2019.

show version
SONiC Software Version: SONiC.HEAD.254-dirty-20190330.203936
Distribution: Debian 9.8
Kernel: 4.9.0-8-2-amd64
Build commit: 9c83b54
Build date: Sat Mar 30 20:47:57 UTC 2019
Built by: johnar@jenkins-worker-4
> hostname 
cab18-3-switch3
> docker exec -it syncd hostname
sonic
> docker exec -it dhcp_relay hostname
sonic
..........

@jleveque
Copy link
Contributor

I don't have any machines running that exact build. However, I just tested on a machine running a newer build from April 11 (commit: sonic-net/sonic-buildimage@0a6dd88), and running hostname inside Docker containers returns the same, proper hostname as the base image.

@pyadvichuk
Copy link
Author

I don't have any machines running that exact build. However, I just tested on a machine running a newer build from April 11 (commit: Azure/sonic-buildimage@0a6dd88), and running hostname inside Docker containers returns the same, proper hostname as the base image.

@jleveque could you please have a look at #2806 as it looks i've found the root couse of issue we've faced.

@pyadvichuk pyadvichuk closed this May 2, 2019
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.

3 participants