Skip to content

Conversation

motinsa
Copy link
Contributor

@motinsa motinsa commented Dec 29, 2024

Context

The create.go file contained a TODO to formalize the way network
options are loaded into internalLabels. The previous implementation
manually assigned each field, which made it harder to maintain and extend.

Solution

  • Introduced a new method LoadNetOpts in the internalLabels struct.
  • Encapsulated the logic for loading network options, improving
    maintainability and extensibility.

Impact

This change is backward-compatible and does not affect existing functionality.

Signed-off-by: Pedro Aibar [email protected]

@@ -732,6 +726,16 @@ func withInternalLabels(internalLabels internalLabels) (containerd.NewContainerO
return containerd.WithAdditionalContainerLabels(m), nil
}

// LoadNetOpts loads network options into InternalLabels.
func (il *internalLabels) LoadNetOpts(opts types.NetworkOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

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

LoadNetOpts is a public function. Could you please clarify why it is necessary for this function to be public rather than private?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making LoadNetOpts public was unintentional. I've already updated the function to be private to align with its intended use case.

Copy link
Member

@fahedouch fahedouch left a comment

Choose a reason for hiding this comment

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

Thanks

@fahedouch fahedouch added this to the v2.0.3 milestone Jan 10, 2025
@fahedouch fahedouch merged commit 751b33b into containerd:main Jan 10, 2025
30 checks passed
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.

3 participants