Skip to content

Conversation

@Igor-splunk
Copy link
Collaborator

Description

What does this PR have in it?

Key Changes

Highlight the updates in specific files

Testing and Verification

How did you test these changes? What automated tests are added?

Related Issues

Jira tickets, GitHub issues, Support tickets...

PR Checklist

  • Code changes adhere to the project's coding standards.
  • Relevant unit and integration tests are included.
  • Documentation has been updated accordingly.
  • All tests pass locally.
  • The PR description follows the project's guidelines.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 30, 2026

CLA Assistant Lite bot CLA Assistant Lite bot All contributors have signed the COC ✍️ ✅

if licenseInfo.Status == "EXPIRED" {
eventPublisher.Warning(ctx, "LicenseExpired",
fmt.Sprintf("License '%s' has expired", licenseName))
scopedLog.Info("Detected expired license", "licenseName", licenseName, "title", licenseInfo.Title)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it be Info?

@coveralls
Copy link
Collaborator

coveralls commented Jan 30, 2026

Pull Request Test Coverage Report for Build 21901106831

Details

  • 47 of 67 (70.15%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 86.117%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/splunk/enterprise/licensemanager.go 47 50 94.0%
pkg/splunk/client/enterprise.go 0 17 0.0%
Files with Coverage Reduction New Missed Lines %
pkg/splunk/enterprise/afwscheduler.go 1 92.72%
Totals Coverage Status
Change from base Build 21866948919: -0.1%
Covered Lines: 10806
Relevant Lines: 12548

💛 - Coveralls

var pod corev1.Pod
err := client.Get(ctx, namespacedName, &pod)
if err != nil {
// Pod might not exist yet, which is normal during initial creation
Copy link
Collaborator

@kubabuczak kubabuczak Feb 9, 2026

Choose a reason for hiding this comment

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

Shouldn't we log something on return?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Those tests look like they could be a table tests.
Which API call will fail in case of expected alerts? Shouldn't we mock it? I would prefer to have some positive test case


// checkLicenseRelatedPodFailures checks license status via Splunk API
// and publishes warning event when expired license is detected
func checkLicenseRelatedPodFailures(ctx context.Context, client splcommon.ControllerClient, cr *enterpriseApi.LicenseManager, statefulSet *appsv1.StatefulSet, eventPublisher *K8EventPublisher) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just take the eventPublisher from the context?

scopedLog := reqLogger.WithName("checkLicenseRelatedPodFailures")

// Check if pod is ready before attempting API call
podName := fmt.Sprintf("%s-0", statefulSet.GetName())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to only check it for one pod?

}

// Check for license-related pod failures before updating
checkLicenseRelatedPodFailures(ctx, client, cr, statefulSet, eventPublisher)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we want to throw an error if sth goes wrong?

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.

4 participants