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

Remove deprecated fields for v2beta1 APIs #4549

Merged
merged 1 commit into from
Jan 26, 2025

Conversation

cyclinder
Copy link
Collaborator

Thanks for contributing!

Notice:

What issue(s) does this PR fix:

Fixes #

Special notes for your reviewer:

As a part of #4301

@cyclinder cyclinder added the release/feature-new release note for new feature label Jan 8, 2025
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 75.25%. Comparing base (655c389) to head (68ba612).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
pkg/podmanager/utils.go 80.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4549      +/-   ##
==========================================
- Coverage   75.69%   75.25%   -0.45%     
==========================================
  Files          56       56              
  Lines        6760     6760              
==========================================
- Hits         5117     5087      -30     
- Misses       1428     1462      +34     
+ Partials      215      211       -4     
Flag Coverage Δ
unittests 75.25% <80.00%> (-0.45%) ⬇️

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

Files with missing lines Coverage Δ
pkg/podmanager/utils.go 52.77% <80.00%> (ø)

... and 2 files with indirect coverage changes

// ChainCNIJsonData is used to configure the configuration of chain CNI.
// format in json.
// +kubebuilder:validation:Optional
ChainCNIJsonData []string `json:"chainCNIJsonData"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

CustomCNIConfig
层级尴尬

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这是 ChainCNIJsonData , 用于配置 chainCNI,customCNIConfig 是另外的字段

// The RDMA resource name of the nic. the RDMA resource is often reported to kubelet by the
// k8s-rdma-shared-dev-plugin. when it is not empty and spiderpool podResourceInject feature
// is enabled, spiderpool can automatically inject it into the container's resources via webhook.
RdmaResourceName string `json:"rdmaResourceName"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个怎么不是 omitempty

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

应该是 omitempty

// The RDMA resource name of the nic. the RDMA resource is often reported to kubelet by the
// k8s-rdma-shared-dev-plugin. when it is not empty and spiderpool podResourceInject feature
// is enabled, spiderpool can automatically inject it into the container's resources via webhook.
RdmaResourceName string `json:"rdmaResourceName"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

omitempty

// +kubebuilder:validation:Required
// The SR-IOV RDMA resource name of the SpiderMultusConfig. the SR-IOV RDMA resource is often
// reported to kubelet by the sriov-device-plugin.
ResourceName string `json:"resourceName"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

omitempty

// +kubebuilder:validation:Optional
// +kubebuilder:deprecated:reason="This field is deprecated and will be removed in future versions."
// DEPRECATED, use RdmaIsolation flled instead.
EnableRdma bool `json:"enableRdma"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

why? 这是官方支持的

// it ensures isolation of RDMA traffic from other workloads in the system by moving
// the associated RDMA interfaces of the provided network interface to the container's
// network namespace path.
RdmaIsolation bool `json:"rdmaIsolation"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

omitempty

// +kubebuilder:validation:Optional
// +kubebuilder:default=false
// Enforces ib-sriov-cni to work with ib-kubernetes.
IbKubernetesEnabled *bool `json:"ibKubernetesEnabled,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

EnableIbKubernetes

@cyclinder cyclinder force-pushed the bump_apis branch 2 times, most recently from 301b2e6 to 02cb8d7 Compare January 23, 2025 08:06
@cyclinder cyclinder requested a review from windsonsea as a code owner January 23, 2025 08:06
@cyclinder
Copy link
Collaborator Author

可以忽略 Auto Golang Lint And Unittest 错误,因为修改了 vendor

@cyclinder cyclinder force-pushed the bump_apis branch 10 times, most recently from a3bfdf5 to d632b07 Compare January 24, 2025 14:12
@cyclinder
Copy link
Collaborator Author

Ignore Auto MarkDown Lint / Markdown Lint, #4560 Will fix it.

@cyclinder cyclinder changed the title update apis to v2beta2 Remove deprecated fields for v2beta1 APIs Jan 26, 2025
// +kubebuilder:validation:Optional
// enable share rdma for macvlan
EnableRdma bool `json:"enableRdma"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

我记得,这个 是 移除了 ? 还是改名了

// +kubebuilder:validation:Optional
// enable share rdma for ipvlan
EnableRdma bool `json:"enableRdma"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个

@weizhoublue
Copy link
Collaborator

CI

@cyclinder
Copy link
Collaborator Author

CI

Ignore Auto MarkDown Lint / Markdown Lint, #4560 Will fix it.

RdmaResourceName string `json:"rdmaResourceName"`
// The RDMA resource name of the nic. the RDMA resource is often reported to kubelet by the
// k8s-rdma-shared-dev-plugin. when it is not empty and spiderpool podResourceInject feature
// is enabled, spiderpool can automatically inject it into the container's resources via webhook.
Copy link
Collaborator

Choose a reason for hiding this comment

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

omitempty 都是定义为 指针的

// The RDMA resource name of the nic. the RDMA resource is often reported to kubelet by the
// k8s-rdma-shared-dev-plugin. when it is not empty and spiderpool podResourceInject feature
// is enabled, spiderpool can automatically inject it into the container's resources via webhook.
RdmaResourceName string `json:"rdmaResourceName,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

同理

@cyclinder cyclinder force-pushed the bump_apis branch 2 times, most recently from a2ba24a to d177e3b Compare January 26, 2025 08:02
Signed-off-by: Cyclinder Kuo <[email protected]>
@cyclinder cyclinder merged commit 938eeb7 into spidernet-io:main Jan 26, 2025
57 checks passed
@cyclinder cyclinder deleted the bump_apis branch January 26, 2025 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/feature-new release note for new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants