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.

Sign up for GitHub

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

Jump to bottom

feat : Add helper function to get all operator pods #2130

Merged
merged 5 commits into from
Jun 18, 2024
Merged

feat : Add helper function to get all operator pods #2130

merged 5 commits into from
Jun 18, 2024

Conversation

bnshr
Copy link
Contributor

@bnshr bnshr commented Jun 3, 2024 •
edited
Loading

Issue JIRA Link

Includes

  • Logic to get all pods managed by the operators under test from the namespaces mentioned through targetNamespaces in CSV. The client code gets this from env.CSVToPodListMap
  • Refactoring of the retrieval of owner references code (moved from provider to podhelper

NOTE
To be used in this PR

sebrandon1
sebrandon1 reviewed Jun 3, 2024
View reviewed changes
Copy link
Member

@sebrandon1 sebrandon1 left a comment

Choose a reason for hiding this comment

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

One comment about the import.

Also we probably should have some simple unit tests wrapping these.

cnf-certification-test/operator/util.go Outdated Show resolved Hide resolved
@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 480515a to 670c21f Compare June 3, 2024 15:52
@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 670c21f to 4ed083d Compare June 3, 2024 16:04
@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 4ed083d to 68ccb00 Compare June 3, 2024 16:05
@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 68ccb00 to 0c38c19 Compare June 3, 2024 16:09
@bnshr bnshr marked this pull request as draft June 3, 2024 16:14
@dcibot
Copy link
Collaborator

dcibot commented Jun 3, 2024

from change #2130:

@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 0c38c19 to bdfa5b3 Compare June 4, 2024 10:28
@bnshr bnshr marked this pull request as ready for review June 4, 2024 10:35
@bnshr bnshr removed the dci-disable label Jun 4, 2024
@dcibot
Copy link
Collaborator

dcibot commented Jun 4, 2024

from change #2130:

sebrandon1
sebrandon1 reviewed Jun 4, 2024
View reviewed changes
Copy link
Member

@sebrandon1 sebrandon1 left a comment

Choose a reason for hiding this comment

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

Some more comments

cnf-certification-test/operator/util_test.go Outdated Show resolved Hide resolved
cnf-certification-test/operator/util.go Outdated Show resolved Hide resolved
cnf-certification-test/operator/util.go Outdated Show resolved Hide resolved
@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch 3 times, most recently from ae7e345 to 9def379 Compare June 5, 2024 12:53
@dcibot
Copy link
Collaborator

dcibot commented Jun 5, 2024

from change #2130:

@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 9def379 to 6c9e256 Compare June 5, 2024 14:18
@dcibot
Copy link
Collaborator

dcibot commented Jun 5, 2024

from change #2130:

sebrandon1
sebrandon1 previously approved these changes Jun 5, 2024
View reviewed changes
@sebrandon1 sebrandon1 dismissed their stale review June 5, 2024 16:05

Found another thing to check in the loop

@sebrandon1
Copy link
Member

@bnshr One more thing we could add here to make everything easier to consume, is to add env.AllOperatorPods as part of our existing pod groupings:

https://github.com/test-network-function/cnf-certification-test/blob/main/pkg/provider/provider.go#L74-L77

Just add:

// Pod Groupings
Pods      []*Pod                 `json:"testPods"`
DebugPods map[string]*corev1.Pod // map from nodename to debugPod
AllPods   []*Pod                 `json:"AllPods"`
AllOperatorPods []*Pod `json:"AllOperatorPods"`           <---------------

We could just easily call the funcs you have created here to collect all of the pods during the autodiscover process.

@bnshr
Copy link
Contributor Author

bnshr commented Jun 5, 2024

@bnshr One more thing we could add here to make everything easier to consume, is to add env.AllOperatorPods as part of our existing pod groupings:

https://github.com/test-network-function/cnf-certification-test/blob/main/pkg/provider/provider.go#L74-L77

Just add:

// Pod Groupings
Pods      []*Pod                 `json:"testPods"`
DebugPods map[string]*corev1.Pod // map from nodename to debugPod
AllPods   []*Pod                 `json:"AllPods"`
AllOperatorPods []*Pod `json:"AllOperatorPods"`           <---------------

We could just easily call the funcs you have created here to collect all of the pods during the autodiscover process.

Makes sense, as I can see that 4 new test cases are created by Shimrit.

@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 6c9e256 to ac544de Compare June 11, 2024 11:54
@dcibot
Copy link
Collaborator

dcibot commented Jun 11, 2024

from change #2130:

@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch 2 times, most recently from 4e7262f to 8cdd73c Compare June 11, 2024 13:11
@dcibot
Copy link
Collaborator

dcibot commented Jun 13, 2024

from change #2130:

sebrandon1
sebrandon1 reviewed Jun 13, 2024
View reviewed changes
Copy link
Member

@sebrandon1 sebrandon1 left a comment

Choose a reason for hiding this comment

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

Some comments

pkg/podhelper/podhelper_test.go Outdated Show resolved Hide resolved
pkg/autodiscover/autodiscover.go Outdated Show resolved Hide resolved
@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from 66b6167 to dee930b Compare June 13, 2024 14:09
@dcibot
Copy link
Collaborator

dcibot commented Jun 13, 2024

from change #2130:

edcdavid
edcdavid reviewed Jun 13, 2024
View reviewed changes
pkg/provider/provider.go Outdated Show resolved Hide resolved
@sebrandon1 sebrandon1 dismissed their stale review June 13, 2024 19:35

Additional changes incoming

@bnshr bnshr marked this pull request as draft June 14, 2024 11:56
@bnshr bnshr marked this pull request as ready for review June 14, 2024 12:28
@bnshr bnshr removed the dci-disable label Jun 14, 2024
@dcibot
Copy link
Collaborator

dcibot commented Jun 14, 2024

from change #2130:

@bnshr bnshr force-pushed the retrieve-operator-managed-resources branch from a2f534b to 6105c0f Compare June 18, 2024 12:02
@dcibot
Copy link
Collaborator

dcibot commented Jun 18, 2024

from change #2130:

  • ERROR no DCI job found

@bnshr
Copy link
Contributor Author

bnshr commented Jun 18, 2024

from change #2130:

  • ERROR no DCI job found

There is no change related to DCI job. Any idea why DCI job is missing?

@sebrandon1
Copy link
Member

@bnshr Sometimes that happens if you aren't fully rebased on main.

@bnshr
Copy link
Contributor Author

bnshr commented Jun 18, 2024

@bnshr Sometimes that happens if you aren't fully rebased on main.

I see. Yes, the branch was not rebased until today. Hope this time it would work.

@dcibot
Copy link
Collaborator

dcibot commented Jun 18, 2024

from change #2130:

@sebrandon1 sebrandon1 merged commit 50389f5 into main Jun 18, 2024
25 checks passed
@sebrandon1 sebrandon1 deleted the retrieve-operator-managed-resources branch June 18, 2024 20:11
@dcibot
Copy link
Collaborator

dcibot commented Jun 18, 2024

from change #2130:

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

@edcdavid edcdavid edcdavid left review comments

@sebrandon1 sebrandon1 sebrandon1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bnshr @dcibot @sebrandon1 @edcdavid