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.

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

Support specifying the transfer net by configmap #1241

Closed
wants to merge 2 commits into from

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Dec 2, 2024

The k8s.v1.cni.cncf.org/networks annotation supports a json value with a list of NetworkSelectionElements that contain additional configuration for the secondary network attachment, including a default route.

This change allows the user to specify the transfer network by referencing a ConfigMap that contains a NetworkSelectionElement. The Plan CR already uses an ObjectReference type for the transferNetwork field, so we can take advantage of the kind field on the ObjectReference to specify that a ConfigMap is being used without having to make any changes to the Plan CR. The transfer network validation has been expanded to check that the ConfigMap is in the correct format and that the NAD the ConfigMap refers to exists.

To maintain compatibility with previous versions of Forklift, the original behavior (using the v1.multus-cni.io/default-network annotation) is preserved if a ConfigMap is not specified in the Plan.

kind: ConfigMap
apiVersion: v1
metadata:
  name: secondary-net-config
  namespace: default
data:
  interface: net2
  name: secondary-net
  namespace: default
  default-route: [...]
spec:
  targetNamespace: default
  transferNetwork:
    kind: ConfigMap
    name: secondary-net-config
    namespace: default
  vms:
    - id: vm-1074
      name: slucidi-mtv

The k8s.v1.cni.cncf.org/networks annotation supports
a json value with a list of NetworkSelectionElements
that contain additional configuration for the secondary
network attachment. This change allows the user to
specify the transfer network by referencing a ConfigMap
that contains a NetworkSelectionElement.

Signed-off-by: Sam Lucidi <[email protected]>
@mansam mansam marked this pull request as ready for review December 2, 2024 23:20
@mansam mansam requested review from mnecas and yaacov as code owners December 2, 2024 23:20
Copy link

sonarcloud bot commented Dec 2, 2024

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 106 lines in your changes missing coverage. Please review.

Project coverage is 15.55%. Comparing base (fb5b40e) to head (f92be76).

Files with missing lines Patch % Lines
pkg/controller/plan/validation.go 0.00% 66 Missing ⚠️
pkg/controller/plan/kubevirt.go 0.00% 40 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1241      +/-   ##
==========================================
- Coverage   15.62%   15.55%   -0.07%     
==========================================
  Files         112      112              
  Lines       23123    23215      +92     
==========================================
  Hits         3612     3612              
- Misses      19226    19318      +92     
  Partials      285      285              
Flag Coverage Δ
unittests 15.55% <0.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mansam
Copy link
Contributor Author

mansam commented Dec 3, 2024

Superceded by #1244

@mansam mansam closed this Dec 3, 2024
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.

2 participants