Add "acn" cli tool to install and manage Azure CNI #688
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
/azp run |
|
No pipelines are associated with this pull request. |
tamilmani1989
previously requested changes
Oct 9, 2020
| apiVersion: apps/v1 | ||
| kind: DaemonSet | ||
| metadata: | ||
| name: azure-cni-installer-agent |
Member
There was a problem hiding this comment.
is it installer or manager? im fine with anything but can we follow same convention everywhere
cni/installer/install.go
Outdated
|
|
||
| func (i *installerConfig) SetCNIMode(cniMode string) error { | ||
| // get paths for singletenancy and multitenancy | ||
| if cniMode != "" { |
Member
There was a problem hiding this comment.
one more check if ipv6mode is set, then mode can be only bridge..can;t be transparent
cni/installer/logwatcher.go
Outdated
| "gopkg.in/fsnotify.v1" | ||
| ) | ||
|
|
||
| func follow(filename string) error { |
Member
There was a problem hiding this comment.
what this package is for? can add brief comment
Member
Author
|
/azp run |
|
No pipelines are associated with this pull request. |
matmerr
changed the title
vakalapa
reviewed
Oct 13, 2020
vakalapa
reviewed
Oct 13, 2020
vakalapa
reviewed
Oct 13, 2020
Contributor
vakalapa
left a comment
vakalapa
left a comment
There was a problem hiding this comment.
Added a few comments. Mostly looks good.
acncli/cmd/generate.go
Outdated
| func GenerateConflistCmd() *cobra.Command { | ||
| var cmd = &cobra.Command{ | ||
| Use: "conflist", | ||
| Short: fmt.Sprintf("Retrieves the logs of %s binary", c.AzureCNIBin), |
Contributor
There was a problem hiding this comment.
Not the right description i suppose ?
acncli/cmd/generate.go
Outdated
| fmt.Printf("%+v", viper.GetBool(c.FlagFollow)) | ||
| // this loop exists for when the logfile gets rotated, and tail loses the original file | ||
| for { | ||
| t, err := tail.TailFile(viper.GetString(c.FlagLogFilePath), tail.Config{Follow: viper.GetBool(c.FlagFollow)}) |
acncli/cmd/generate.go
Outdated
| ) | ||
|
|
||
| // installCmd can register an object | ||
| func GenerateCmd() *cobra.Command { |
Contributor
There was a problem hiding this comment.
Where is this Cmd being used ? Also comment on the top.
acncli/cmd/logs.go
Outdated
| "github.com/spf13/viper" | ||
| ) | ||
|
|
||
| // installCmd can register an object |
acncli/cmd/manager.go
Outdated
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // installCmd can register an object |
acncli/cmd/logs.go
Outdated
| }} | ||
|
|
||
| cmd.Flags().BoolP(c.FlagFollow, "f", c.DefaultToggles[c.FlagFollow], fmt.Sprintf("Follow the log file, similar to 'tail -f'")) | ||
|
|
| } | ||
|
|
||
| func InstallLocal(installerConf InstallerConfig) error { | ||
| fmt.Printf("📁 - Checking if destination bin directory (%s) exists...\n", installerConf.DstBinDir) |
Contributor
There was a problem hiding this comment.
<!comment> Great idea with the emojis, are these printed in stdout or logfile ? If logfile are we sure the form exists ? If stdout, some terminals may not have support for emojis.
| value: linux | ||
| - name: CNI_TYPE | ||
| value: singletenancy | ||
| - name: CNI_MODE |
Contributor
There was a problem hiding this comment.
Two different naming conventions for envs in agent and master, agent has "Azure_*".
| ) | ||
|
|
||
| func SetOrUseDefault(setValue, defaultValue string) string { | ||
| if setValue == "" { |
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov Report
@@ Coverage Diff @@
## master #688 +/- ##
==========================================
- Coverage 38.60% 38.56% -0.05%
==========================================
Files 78 79 +1
Lines 10433 10447 +14
==========================================
+ Hits 4028 4029 +1
- Misses 5913 5926 +13
Partials 492 492 |
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
vakalapa
approved these changes
Oct 20, 2020
| return err | ||
| } | ||
|
|
||
| // only allow windows and linux binaries |
Contributor
There was a problem hiding this comment.
Comment remove or change to //only allow Single or Multi tenancy
| return err | ||
| } | ||
|
|
||
| // only allow windows and linux binaries |
matmerr
dismissed
tamilmani1989’s stale review
Handle extra checking can be done in a seperate PR
matmerr
added a commit
that referenced
this pull request
Nov 12, 2020
* tail azure-vnet.logs * dockerfile update * installer fixes * remove external deps * move to cli design * manager cmd * update vendor * minor fixes * logs * update makefile * Update manager-master.yaml * Update manager-agent.yaml
matmerr
added a commit
that referenced
this pull request
Nov 12, 2020
* tail azure-vnet.logs * dockerfile update * installer fixes * remove external deps * move to cli design * manager cmd * update vendor * minor fixes * logs * update makefile * Update manager-master.yaml * Update manager-agent.yaml
neaggarwMS
pushed a commit
to neaggarwMS/azure-container-networking
that referenced
this pull request
Nov 13, 2020
* tail azure-vnet.logs * dockerfile update * installer fixes * remove external deps * move to cli design * manager cmd * update vendor * minor fixes * logs * update makefile * Update manager-master.yaml * Update manager-agent.yaml
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reason for Change:
Start a long lived process to tail the azure-vnet.log and output to stdout of the installer container
Issue Fixed:
Requirements:
Notes: