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

chore: add full NCP CR example #993

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Conversation

rollandf
Copy link
Member

No description provided.

@coveralls
Copy link
Collaborator

coveralls commented Jul 15, 2024

Pull Request Test Coverage Report for Build 10053779049

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.4%) to 64.172%

Files with Coverage Reduction New Missed Lines %
pkg/state/state_hostdevice_network.go 2 78.38%
controllers/hostdevicenetwork_controller.go 2 89.77%
controllers/nicclusterpolicy_controller.go 3 79.08%
Totals Coverage Status
Change from base Build 9987157196: 0.4%
Covered Lines: 3267
Relevant Lines: 5091

💛 - Coveralls

@noama-nv
Copy link

LGTM

@@ -57,3 +57,40 @@ spec:
}
]
}
sriovDevicePlugin:

Choose a reason for hiding this comment

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

@rollandf secondaryNetwork ipoib is also applicable on openshift

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@rollandf rollandf force-pushed the full-ncp-cr branch 2 times, most recently from fb62abf to c4e811e Compare July 22, 2024 06:44
@ykulazhenkov ykulazhenkov self-requested a review July 22, 2024 06:45
ykulazhenkov
ykulazhenkov previously approved these changes Jul 22, 2024
Copy link
Collaborator

@ykulazhenkov ykulazhenkov left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +14 to +18
#
# ##### Note #####
# This example contains all the components supported as a reference.
# User should edit the example and keep only the required components.
#
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#
# ##### Note #####
# This example contains all the components supported as a reference.
# User should edit the example and keep only the required components.
#

nit: do we need this example text or should we rather remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

This example is a reference, that was asked from QA team to easily get the components versions, taking into account that in the future these versions will not be available in the Helm values.
This configuration probably is not a real life scenario.

@rollandf
Copy link
Member Author

/retest-blackduck_scan

@rollandf
Copy link
Member Author

/retest-blackduck_scan

2 similar comments
@rollandf
Copy link
Member Author

/retest-blackduck_scan

@rollandf
Copy link
Member Author

/retest-blackduck_scan

@@ -0,0 +1,118 @@
# Copyright 2020 NVIDIA
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, update copyright header as at [1] and fix the year

[1]

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@rollandf
Copy link
Member Author

/retest-image_scan

@adrianchiris adrianchiris merged commit 884124f into Mellanox:master Jul 25, 2024
16 checks passed
@rollandf rollandf deleted the full-ncp-cr branch August 27, 2024 08:55
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.

7 participants