Skip to content

Ipamutils: various cleanups and refactor#2593

Closed
thaJeztah wants to merge 4 commits intomoby:masterfrom
thaJeztah:ipamutils_cleanup
Closed

Ipamutils: various cleanups and refactor#2593
thaJeztah wants to merge 4 commits intomoby:masterfrom
thaJeztah:ipamutils_cleanup

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

  • ipamutils: use RWMutex to allow concurrent reads
  • ipamutils: reformat for readability, and improve godoc
  • ipamutils: un-export global variables, and use accessors instead
    The GetLocalScopeDefaultNetworks() and GetGlobalScopeDefaultNetworks() protect these variables with a lock, but various parts in the code were directly accessing the variables without locks.
  • ipamutils: generate default pools on first use, instead of init()
    This package defines the NetworkToSplit type. Curently importing the package also initializes the default address pools (which may not be needed, e.g., when used in the client.
    This patch changes initialization to happen on first use, instead of init(), to allow the package to be imported without initializing these pools.

The `GetLocalScopeDefaultNetworks()` and `GetGlobalScopeDefaultNetworks()`
protect these variables with a lock, but various parts in the code
were directly accessing the variables without locks.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
This package defines the `NetworkToSplit` type. Curently importing
the package also initializes the default address pools (which may
not be needed, e.g., when used in the client.

This patch changes initialization to happen on first use, instead
of `init()`, to allow the package to be imported without initializing
these pools.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member Author

@arkodg @cpuguy83 ptal

Comment thread ipamutils/utils.go
Size int `json:"size"`
}

func init() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn't init easier to reason with than initDefaults ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You mean "using an init() function"? Or to rename the initDefaults (sync.Once) to init? (not sure if the latter works; I suspect init is somewhat "reserved"))

@corhere
Copy link
Copy Markdown
Collaborator

corhere commented Mar 31, 2023

@corhere corhere closed this Mar 31, 2023
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