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

Pull underlying capabilities up to 00-multus.conf #545

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

clementnuss
Copy link
Contributor

As discussed in issue #544 the nested plugin capabilities are not integrated in the 00-multus.conf file generated by the images/entrypoint.sh file.
I implemented the parsing of the underlying master plugin json configuration file with python, to retrieve the nested capabilities and to integrate them in the top-level section of the 00-multus.conf file (because it is not sufficient to have capabilities declared in the delegates section of the CNI config file.)

The python script for analyzing underlying plugins capabilities is compatible with Python 2 and Python 3, it could be a bit shorter if I used only Python 3 syntax, but I noticed that the Dockerfile is based on a CentOS version that uses Phyton 2.
When no capabilities in the underlying plugins, then nothing gets added to the 00-multus.conf file.

@coveralls
Copy link

coveralls commented Aug 5, 2020

Pull Request Test Coverage Report for Build 195949001

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.997%

Totals Coverage Status
Change from base Build 185831388: 0.0%
Covered Lines: 1082
Relevant Lines: 1524

💛 - Coveralls

@clementnuss clementnuss force-pushed the master branch 2 times, most recently from f14bad4 to 9d5c91d Compare August 5, 2020 08:07
@clementnuss
Copy link
Contributor Author

clementnuss commented Aug 5, 2020

Sorry for the multiple force-push: I introduced a modification that improves the parsing of the capabilities, and first commited a vim file instead of my modification, and when I force pushed a new commit with the correct file I forgot to put a title for the commit 🤦‍♂️😅

Finally, I have reverted both commits and replaced them by a single one with a shorter commit message.

Using Python, we analyze the content of the master plugin configuration,
and we integrate any capabilities from the underlying chained plugins
declaration. We only pull enabled capabilties from the underlying
(chained) plugin definitions.

Fixes intel#544

Signed-off-by: <Clement.Nussbaumer@Swisscom.com>
@s1061123
Copy link
Member

s1061123 commented Aug 6, 2020

Thank you for your PR, @clementnuss !

It looks good to me. Let's wait a few days for other comments and merge it.

Copy link
Member

@dougbtv dougbtv left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution!

@dougbtv dougbtv merged commit be492e1 into k8snetworkplumbingwg:master Aug 6, 2020
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.

4 participants