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

Convert dhcp-relay docker to buster #4671

Merged
merged 9 commits into from
Jun 22, 2020

Conversation

joyas-joseph
Copy link
Contributor

Signed-off-by: Joyas Joseph joyas_joseph@dell.com

- Why I did it
This is an enhancement.

- How I did it
Code change.
Added a small patch to get the compilation work under buster environment.

- How to verify it
Build the docker: BLDENV=buster make -f Makefile.work target/docker-dhcp-relay.gz
docker exec dhcp_relay cat /etc/apt/sources.list

I have verified that the DHCP request are being relayed to the server. I haven't tested with a real DHCP server.

- Description for the changelog
Convert dhcp-relay docker to buster

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

Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
@@ -11,3 +11,4 @@
0010-CVE-2018-5732.patch
0008-interface-name-maxlen-crash.patch
0012-Don-t-skip-down-interfaces-when-discovering-interfac.patch
0013-buster-build.patch
Copy link
Contributor

@jleveque jleveque May 29, 2020

Choose a reason for hiding this comment

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

Rather than patching isc-dhcp source to forward-port 4.3.5-2 to Buster, I think it is better to upgrade the isc-dhcp source to the version available in buster (4.4.1-2). However, this may require more extensive rework of existing patches, and thus more extensive testing.

@lguohan: What is your opinion here? Should we do that here, or go with this approach temporarily and upgrade isc-dhcp in a future PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

we did the same thing for snmp to upgrade to buster. @joyas-joseph, can you do the upgrade in this pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quick update - of the 12 patches, only 3 (patches 1,6 and 12) applied cleanly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

any further update on this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The remaining 9 patches need to be manually reworked and also extensively tested. This is going to take time.

For SNMP case; since it was only a minor version update, we didnt have to rework any of the patches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have reworked the patches and pushed the changes.
I haven't tested option-82 though.

I would like @jleveque to look at the patches, especially the changes to common/discover.c

Update libevent dependency for dhcpmon to 2.1-6

Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
@lguohan
Copy link
Collaborator

lguohan commented Jun 4, 2020

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Jun 4, 2020

retest mellanox please

1 similar comment
@lguohan
Copy link
Collaborator

lguohan commented Jun 9, 2020

retest mellanox please

@lguohan
Copy link
Collaborator

lguohan commented Jun 9, 2020

retest vsimage please

lguohan
lguohan previously approved these changes Jun 9, 2020
@lguohan
Copy link
Collaborator

lguohan commented Jun 12, 2020

retest vsimage please

Remove the patch as it is no longer required.

Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
Comment on lines 12 to 9
0008-interface-name-maxlen-crash.patch
0012-Don-t-skip-down-interfaces-when-discovering-interfac.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that you've removed four patches, can you please rename 0012-Don-t-skip-down-interfaces-when-discovering-interfac.patch to 0008-Don-t-skip-down-interfaces-when-discovering-interfac.patch?

Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
@lguohan
Copy link
Collaborator

lguohan commented Jun 22, 2020

retest vsimage please

@lguohan lguohan merged commit b48d274 into sonic-net:master Jun 22, 2020
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