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

Provide a way to use PodController object to create PodRunner object #2229

Merged
merged 28 commits into from
Aug 10, 2023
Merged

Provide a way to use PodController object to create PodRunner object #2229

merged 28 commits into from
Aug 10, 2023

Conversation

chaitalisg
Copy link
Contributor

@chaitalisg chaitalisg commented Jul 26, 2023 •
edited

Change Overview

This PR

  • exports methods and fields to utilize the fake pod controller processor outside Kanister. It would help in simulating pod creation and deletion errors in unit tests.
  • introduces fake pod controller and fake pod command executor. This would help in simulating pod execution errors in unit tests.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • fixes #issue-number

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

Unit tests in progress.

@github-actions
Copy link
Contributor

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Jul 26, 2023
chaitalisg
chaitalisg commented Jul 26, 2023
View reviewed changes
pkg/kube/pod_runner.go Outdated Show resolved Hide resolved
@chaitalisg chaitalisg changed the title add comments to explain the issue better Provide a way to use PodController object to create PodRunner object Jul 27, 2023
@chaitalisg chaitalisg marked this pull request as ready for review August 1, 2023 18:48
e-sumin
e-sumin approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@e-sumin e-sumin left a comment

Choose a reason for hiding this comment

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

Generally looks good for me, only one thing is not clear for me - why you've added suffix PCP for couple methods WaitForPodReadyPCP ?

Kanister automation moved this from In Progress to Reviewer approved Aug 8, 2023
@chaitalisg
Copy link
Contributor Author

Generally looks good for me, only one thing is not clear for me - why you've added suffix PCP for couple methods WaitForPodReadyPCP ?

I have added the prefix to the methods provided by the PodControllerProcessor interface. I ran into a few issues with the method names being identical for PodController and PodControllerProcessor interfaces.

pavannd1
pavannd1 reviewed Aug 9, 2023
View reviewed changes
pkg/kube/pod_controller.go Outdated Show resolved Hide resolved
pkg/kube/pod_controller.go Outdated Show resolved Hide resolved
pkg/kube/fake_pod_controller.go Outdated Show resolved Hide resolved
pkg/kube/fake_pod_controller.go Show resolved Hide resolved
pkg/kube/fake_pod_controller_processor.go Show resolved Hide resolved
pkg/kube/fake_pod_command_executor.go Show resolved Hide resolved
pkg/kube/fake_pod_command_executor.go Outdated Show resolved Hide resolved
chaitalisg and others added 2 commits August 9, 2023 16:46
@chaitalisg @pavannd1
Apply suggestions from code review
bf39e42 
Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>
@chaitalisg
address feedbacl
d72ce5d
@mergify mergify bot merged commit ae09b1f into master Aug 10, 2023
15 checks passed
@mergify mergify bot deleted the provide-fake-pod-functions branch August 10, 2023 02:16
Kanister automation moved this from Reviewer approved to Done Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@e-sumin e-sumin e-sumin approved these changes

@pavannd1 pavannd1 pavannd1 approved these changes

Assignees
No one assigned
Labels
kueue
Projects
Kanister
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@chaitalisg @pavannd1 @e-sumin