Hostnic testcases - #23
Merged
Merged
Conversation
tkatila
requested changes
Sep 10, 2026
tkatila
left a comment
tkatila
left a comment
Contributor
There was a problem hiding this comment.
Some simplification thoughts.
pfl
force-pushed
the
hostnic_testcases
branch
from
September 15, 2026 10:20
8156da5 to
d83e556
Compare
Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
Verify that the scheme handed to the controller manager knows both the built in Kubernetes types the operator deploys and the NetworkClusterPolicy custom resource. Remove cmd/operator from the list of packages excluded from the test target so that the new test cases run and are accounted for in the coverage profile. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
Move the command line flag declarations into a parseFlags() function that registers them on a caller supplied flag set and returns the parsed values in an options struct. This allows the flag defaults and the parsing to be tested without touching the global flag set of the process. Test the defaults, the overridden values, invalid input and that the zap and klog logging flags end up on the given flag set. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
Move the TLS options shared by the metrics and the webhook server into a tlsOptions() function so that the resulting configuration can be inspected by applying the options to a TLS configuration. Test the minimum and maximum TLS version, the cipher suites and that the negotiated protocols are pinned to http/1.1 unless http/2 is explicitly enabled. Also test that the returned options carry no shared state, as the same options are handed to two servers. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
Move the metrics server configuration into a metricsOptions() function. Test that the bind address is passed through and that the authentication and authorization filter provider is installed exactly when the metrics endpoint is served securely. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
pfl
force-pushed
the
hostnic_testcases
branch
from
September 17, 2026 07:05
d83e556 to
511de3e
Compare
Contributor
Author
|
Lint discoveries also fixed. |
tkatila
reviewed
Sep 17, 2026
Move the reading of the OPERATOR_NAMESPACE and ENABLE_WEBHOOKS environment variables into operatorNamespace() and webhooksEnabled() functions. Test that the namespace falls back to the default one when the variable is empty or unset, and that only the exact value 'false' disables the admission webhooks. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
Split the decision whether an API group list belongs to an OpenShift cluster out of isOpenShift() into a hasOpenShiftGroups() function, leaving only the discovery client plumbing behind. The function can then be tested with plain API group lists and without a cluster. Test the vanilla Kubernetes case, both OpenShift API groups, near miss group names and that isOpenShift() reports the error of a missing cluster instead of claiming a vanilla Kubernetes environment. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
modifyDranetDaemonSet() applies the image and pull policy from the cluster policy to the DRANet container. Add a table covering all four combinations of the two optional overrides and a spec verifying that containers other than DRANet are left untouched. The function does not use the reconciler receiver, so no client is needed here. This brings modifyDranetDaemonSet() to full statement coverage. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
The existing specs call the individual update functions directly, so Reconcile() itself was only exercised with cluster policies that take the removal path. Add specs that reconcile a policy with DRANet installation enabled and verify that - all five DRANet objects are created with the app and owner labels and a controller reference to the cluster policy, - a second reconcile does not touch them, which an unnecessary update would reveal as a changed resource version, - disabling the installation, selecting another configuration type and reconciling without a cluster policy all remove the objects, and - objects owned by another cluster policy are left in place. The in-memory client and the objects to look up are shared as newHostNICReconciler() and hostNICObjects() for use by later specs. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
updateDeviceClass() is the only DRANet object handler that can rename or remove its object while the installation stays enabled. Add specs for the cases the existing ones do not reach: - an unnamed device class falls back to the name in the shipped manifest, - no device class is installed when the cluster policy does not ask for one, and an already installed one is removed, - renaming the device class installs the new one and prunes the previous one, and - a modified device class spec is restored while the labels and the controller reference of the installed object are preserved. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
The update functions fetch the installed object, apply the intended contents on top of the existing metadata and update only if that makes a difference. The existing specs cover creating the objects and the no-change case, so the update branches were never taken. Add specs that modify an installed object and check the result of the following update: - removed cluster role rules are restored, - a repointed cluster role binding gets its role reference and its subject namespaces back, and - a changed DRANet container image is set back to the image of the cluster policy. Labels and the controller reference of the installed objects are expected to survive in all three cases. The last spec documents that an image change is not reverted when the cluster policy does not set an image, as only the settings given in the cluster policy are enforced. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
The webhook specs only covered the gaudi-so configuration type. Add specs for the hostnic-so path: - Default() fills in the default RDMA device class name when the name is empty, keeps an explicitly given name, and leaves the device class absent when none is requested. - Default() does not touch the hostnic spec for other configuration types. - validateSpec() dispatches hostnic-so to validateHostNicSoSpec(), which rejects a device class without a name and, unlike gaudi-so, accepts a policy without a nodeSelector. - Defaulting turns an otherwise rejected spec into a valid one. This brings Default(), validateSpec() and validateHostNicSoSpec() to full statement coverage. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
pfl
force-pushed
the
hostnic_testcases
branch
from
September 17, 2026 14:25
511de3e to
af3dfe2
Compare
tkatila
approved these changes
Sep 21, 2026
| }, | ||
| } | ||
|
|
||
| Expect(nc.ValidateCreate()).Error().To(BeNil()) |
Contributor
There was a problem hiding this comment.
This is fine and no need to modify.
Slightly better format would be Expect(err).NotTo().Have(Occurred()).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve test case coverage mainly for hostnic related code.