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.

Sign up for GitHub

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

Jump to bottom

Make CNI configurable and add Calico as an option #308

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make CNI configurable and add Calico as an option #308

wants to merge 2 commits into from

Conversation

MattiasGees
Copy link
Contributor

This makes CNI configurable so people can start adding other CNI support to it. Calico is also now one of the options to use.
The default is still Canal and it will be picked automatically when no option is given.

xetys
xetys requested changes Dec 26, 2019
View reviewed changes
Copy link
Owner

@xetys xetys left a comment

Choose a reason for hiding this comment

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

And please solve the 2 CC issues.

Thank's for the great contribution!

pkg/clustermanager/cluster.go Outdated Show resolved Hide resolved
pkg/clustermanager/cluster.go Outdated Show resolved Hide resolved
pkg/clustermanager/cluster.go Outdated Show resolved Hide resolved
@xetys
Copy link
Owner

xetys commented Dec 26, 2019

Do you think you can fix the CC issues, too?

@MattiasGees
Copy link
Contributor Author

I have been looking at how to optimise it and not sure how to do it. Pointers are welcome :)

validateClusterCreateFlags only adds 3 lines. Even when I move it to another method it will stay the same

InstallMasters adds 5 lines, but I am also not sure it is worth moving that part to another method

@MattiasGees
Copy link
Contributor Author

@xetys wdyt?

@xetys
Copy link
Owner

xetys commented Mar 2, 2020

I would say ignore CC, but pls resolve the merge conflicts

@MattiasGees
Make CNI configurable and add Calico as an option
4a60fc8 
This makes CNI configurable so people can start adding other CNI support to it. Calico is also now one of the options to use.
The default is still Canal and it will be picked automatically when no option is given.
@MattiasGees
Rename CNI to cni
533c434
@MattiasGees
Copy link
Contributor Author

Merge conflicts have been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xetys xetys xetys requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MattiasGees @xetys