Skip to content

Conversation

@ratnam915
Copy link
Contributor

Redirected from #357 to minimize conflicting changes

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 12, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 12, 2025

@ratnam915: This pull request references SREP-1120 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set.

In response to this:

Redirected from #357 to minimize conflicting changes

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@codecov
Copy link

codecov bot commented Aug 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.15%. Comparing base (7718f12) to head (4fc8473).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #379   +/-   ##
=======================================
  Coverage   57.15%   57.15%           
=======================================
  Files          29       29           
  Lines        2138     2138           
=======================================
  Hits         1222     1222           
  Misses        802      802           
  Partials      114      114           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ratnam915
Copy link
Contributor Author

/test all

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 20, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 29, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 29, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ratnam915
Once this PR has been reviewed and has the lgtm label, please assign rogbas for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ratnam915 ratnam915 force-pushed the feature/SREP-1120 branch 2 times, most recently from 29c04ac to a9d7b38 Compare September 30, 2025 12:35
@ratnam915
Copy link
Contributor Author

ratnam915 commented Sep 30, 2025

Successful execution of test cases carried out by building the e2e test image.

Attached is the output for the same.

(https://github.com/user-attachments/files/22619368/output2.txt)
Screenshot From 2025-09-30 14-46-27

@ritmun
Copy link
Contributor

ritmun commented Sep 30, 2025

@bmeng
Copy link
Contributor

bmeng commented Oct 10, 2025

can we separate the boilerplate update and the code change?

@ratnam915
Copy link
Contributor Author

@bmeng : The boilerplate changes have now been kept seperate.

PR #402 has been raised for the changes

@ratnam915
Copy link
Contributor Author

/test all

@krishvoor
Copy link
Member

/test validate

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 29, 2025

@ratnam915: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

"metadata": map[string]interface{}{
"name": clusterDeploymentName,
"namespace": config.TestNamespace,
"labels": map[string]interface{}{
Copy link
Contributor

Choose a reason for hiding this comment

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

do you know if all the labels here are required?

"hive.openshift.io/version-major": "4",
"hive.openshift.io/version-major-minor": "4.19",
},
"annotations": map[string]interface{}{
Copy link
Contributor

Choose a reason for hiding this comment

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

and also the annotations here as well

}
}

func VerifyMetrics(ctx context.Context, dynamicClient dynamic.Interface, certificateRequestGVR schema.GroupVersionResource, namespace string) (int, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the function name mean? What kind of metric we are verifying?


// Return true if any CertificateRequests exist
return len(crList.Items) > 0
}, pollingDuration, 30*time.Second).Should(gomega.BeTrue(), "CertificateRequest should be created by operator")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to check the name of certificaterequest here?

})

ginkgo.It("should verify certificate operation metrics", func(ctx context.Context) {
ginkgo.GinkgoLogr.Info("=== Test: Verifying Certificate Operation Metrics ===")
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel the metrics here is a bit misleading, as the metrics in kubernetes have the special mean usually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants