chore: start breaking up CNS main #3465
Conversation
Signed-off-by: Evan Baker <[email protected]>
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
PR Overview
This PR is a chore aimed at starting the process of breaking up the large CNS main file into smaller, more manageable components.
- Introduces a new file for version information in cns/service/version.go
- Sets up a helper function to print application version details
Reviewed Changes
| File | Description |
|---|---|
| cns/service/version.go | Adds a helper for printing version details |
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
cns/service/version.go:1
- [nitpick] The file is located in the 'cns/service' directory but declares 'package main'. Consider renaming it (e.g., to 'service') to align with its directory context.
package main
No code changes, only copy/pasting.
eventually - I'm aiming for cobra. But it's very far down the line. |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Pull request closed due to inactivity. |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
rbtr
added
exempt-stale
There was a problem hiding this comment.
Yeah I think once these are also moved then we are good to go:
aks.go
func InitializeMultiTenantController()func pollNodeInfoCRDAndUpdatePlugin()
managed.go ?
type NodeInterrogatorfunc registerNode()func sendRegisterNodeRequest()- These are only used if cns is
Managedchannel mode
cns/restserver/nodesubnet.go
func getPodInfoByIPProvider() could go here, because there's another function func InitalizeNodeSubnet() that is already there, and is only called when channel mode is AzureHost
This actually results in a cycle
And then this one is probably a stretch, should we make an args.go for the var args = acn.ArgumentList{..}?
Super nice to be doing this! Thanks Evan!
|
|
||
| // Prints description and version information. | ||
| func printVersion() { | ||
| fmt.Printf("Azure Container Network Service\n") |
There was a problem hiding this comment.
I'm just wondering if up above, it makes sense also to tear out var args = acn.ArgumentList{..} and also put it into a file like args.go or something
| // Prints description and version information. | ||
| func printVersion() { | ||
| fmt.Printf("Azure Container Network Service\n") | ||
| fmt.Printf("Version %v\n", version) |
There was a problem hiding this comment.
I think we should also take out type NodeInterrogator ,func registerNode() and func sendRegisterNodeRequest() and put them into a file called managed.go, or something, since they are only used if cns is in "managed" mode it seems (not sure really what that is but it could also make this file smaller)
| logger.Printf("Set GlobalPodInfoScheme %v (InitializeFromCNI=%t)", cns.GlobalPodInfoScheme, cnsconfig.InitializeFromCNI) | ||
|
|
||
| err = InitializeCRDState(rootCtx, httpRemoteRestService, cnsconfig) | ||
| err = initializeCRDState(rootCtx, httpRemoteRestService, cnsconfig) |
There was a problem hiding this comment.
Should we also move func InitializeMultiTenantController() into aks.go?
| initialIBNICCount = 0 | ||
| ) | ||
|
|
||
| type cniConflistScenario string |
There was a problem hiding this comment.
Alright, just a few more things we may want to move out of here and into aks.go
the const:
defaultNodeInfoCRDPollInterval (maybe that doesn't really warrant a move)
The function:
func pollNodeInfoCRDAndUpdatePlugin (just the definition of the function not the function call)
no code changes, only rearranging.
starting to break up the huge spaghetti that is
main