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

Add support for multiple assisted-service pods #2148

Closed
wants to merge 1 commit into from
Closed

Add support for multiple assisted-service pods #2148

wants to merge 1 commit into from

Conversation

bkopilov
Copy link
Contributor

@bkopilov bkopilov commented May 9, 2023

Added support for:

  • restart_deployment We can restart the deployment without stop , the restarted managed and creates another replicas and manages the pods creation/termination. will allow us to run tests during agent upgrades as active/active
  • delete_leftovers , due to the way restartAt works we have to clean all leftovers for the next run, before cleanup we remove replicaset with zero or not running pods
  • containers_ready_inside_pod Added support for deep inspection of a pod and verify that inside containers are really marked as ready. also we calculate the total pods plus verify that each one is in ready (from conainter status)

At the end of the test we back to the last good state with a single pod, prefer to do it with oc scale replicas because there are no replicas leftover

@bkopilov
Copy link
Contributor Author

bkopilov commented May 9, 2023

@eliorerz , @osherdp , @lalon4 , @thilzenr@redhat.com

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 9, 2023
@openshift-ci openshift-ci bot requested review from eliorerz and tsorya May 9, 2023 12:46
@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 9, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 9, 2023

Hi @bkopilov. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

@bkopilov
Add support for multiple assisted-service pods
1ee0658 
Added support for:
- restart_deployment
We can restart the deployment without stop , the restarted managed and creates
another replicas and manages the pods creation/termination. will allow us to run
tests during agent upgrades as active/active
- delete_leftovers , due to the way restartAt works we have to clean all leftovers
for the next run, before cleanup we remove replicaset with zero or not running pods
- containers_ready_inside_pod
Added support for deep inspection of a pod and verify that inside containers are
really marked as ready. also we calculate the total pods plus verify that each one
is in ready (from conainter status)

At the end of the test we back to the last good state with a single pod, prefer to do it with
oc scale replicas because there are no replicas leftover
@bkopilov bkopilov force-pushed the agent_restart_support branch from 8296ec8 to 1ee0658 Compare May 29, 2023 12:47
@openshift-ci
Copy link

openshift-ci bot commented May 29, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bkopilov
Once this PR has been reviewed and has the lgtm label, please ask for approval from eliorerz. For more information see the Kubernetes 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

adriengentil
adriengentil reviewed Jun 8, 2023
View reviewed changes
src/assisted_test_infra/test_infra/controllers/assisted_installer_infra_controller.py
Comment on lines +251 to +254
running_pods = list(map(self.containers_ready_inside_pod, pods))
# make sure all pods are running , in restartAt we may have parallel creation/termination
logging.info(f"Check if total pods are equal to running and ready {running_pods} vs {len(pods)}")
return len(pods) == len(running_pods) and all(running_pods)
Copy link
Contributor

Choose a reason for hiding this comment

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

could be simpler to check if the full deployment has been rolled out successfully?

You can get inspiration from here:
https://github.com/fiaas/fiaas-deploy-daemon/pull/47/files#diff-2a85b9b3dfebaeb782d011387395182908f17ba26d491c029850f6ddcae02cecR66-R69

it does the same check as kubectl rollout status ...

@adriengentil
Copy link
Contributor

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 8, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jun 8, 2023

@bkopilov: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-metal-assisted-kube-api-late-binding-single-node 1ee0658 link false /test e2e-metal-assisted-kube-api-late-binding-single-node
ci/prow/e2e-metal-single-node-live-iso 1ee0658 link true /test e2e-metal-single-node-live-iso

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/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 7, 2023
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 7, 2023
@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci bot closed this Nov 7, 2023
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 7, 2023

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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/test-infra repository.

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

@adriengentil adriengentil adriengentil left review comments

@eliorerz eliorerz eliorerz requested changes

@tsorya tsorya Awaiting requested review from tsorya

Assignees

@eliorerz eliorerz

Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bkopilov @adriengentil @openshift-bot @eliorerz