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.

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

Add a field in actionset status to figure out currently running phase #1798

Merged
merged 10 commits into from
Jan 3, 2023

Conversation

viveksinghggits
Copy link
Contributor

@viveksinghggits viveksinghggits commented Dec 16, 2022

Change Overview

This PR adds another field in the actionset status that can be used to figure out the currently running phase of the actionset set.
And also adds that field in the additionalPrinterColumns so that we can get the phase that is currently running in the kubectl output itself.

Every 2.0s: kubectl get actionsets.cr.kanister.io -n kanister                         vivek: Mon Dec 26 19:48:30 2022

NAME           PROGRESS   RUNNINGPHASE   LAST TRANSITION TIME   STATE
backup-mqw49   66.67                     2022-12-26T18:47:32Z   complete
backup-tcqb7   66.67                     2022-12-26T18:47:06Z   complete
backup-vqzq8   33.33                     2022-12-26T18:48:16Z   failed
backup-zxxwd   33.33                     2022-12-26T18:48:00Z   failed

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

@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 Dec 16, 2022
@viveksinghggits viveksinghggits force-pushed the actionsetstatus-onphase branch 3 times, most recently from a06132e to e35ea62 Compare December 19, 2022 15:43
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

Added some questions/suggestions.

pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/phase.go Outdated Show resolved Hide resolved
pkg/customresource/actionset.yaml Outdated Show resolved Hide resolved
@viveksinghggits
Copy link
Contributor Author

Every 2.0s: kubectl get actionsets.cr.kanister.io -n kanister                         vivek: Mon Dec 26 19:48:30 2022

NAME           PROGRESS   RUNNINGPHASE   LAST TRANSITION TIME   STATE
backup-mqw49   66.67                     2022-12-26T18:47:32Z   complete
backup-tcqb7   66.67                     2022-12-26T18:47:06Z   complete
backup-vqzq8   33.33                     2022-12-26T18:48:16Z   failed
backup-zxxwd   33.33                     2022-12-26T18:48:00Z   failed

This commit adds another field in the actionset status that can
be used to figure out the currently running phase of the actionset
set.
Add test to make sure the phases that are in blueprint are actually
being set in the actionset status.progress correctly.
This commit changes the `onPhase` field to `runningPhase`
and also makes sure that `status.Progress.RunningPhase`
is updated with either complete of failed values, based
on the actionset's status, to make sure things are not
confusing.
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

Have some minor suggestions. Otherwise looks good

pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/controller/controller_test.go Outdated Show resolved Hide resolved
pkg/customresource/actionset.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

LGTM

Kanister automation moved this from In Progress to Reviewer approved Dec 29, 2022
docs/tutorial.rst Outdated Show resolved Hide resolved
pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/controller/controller_test.go Outdated Show resolved Hide resolved
pkg/controller/controller_test.go Show resolved Hide resolved
pkg/customresource/actionset.yaml Outdated Show resolved Hide resolved
pkg/customresource/actionset.yaml Outdated Show resolved Hide resolved
@pavannd1 pavannd1 removed the kueue label Dec 30, 2022
@viveksinghggits
Copy link
Contributor Author

viveksinghggits commented Dec 30, 2022

CI failed once because of

FAIL: <autogenerated>:1: PostgreSQL.TestRun

integration_test.go:220:
    c.Assert(err, IsNil)
... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc000f722e8), Stderr:[]uint8(nil)} ("signal: killed")

It seems to be a resource issue. I have re-triggered the CI, if we see this again, we can talk to Github Actions team to know more about the machines where these tests run and what we can do about those.

This document also has details about the runners that are used to run these tests https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources

@mergify mergify bot merged commit 59124c5 into master Jan 3, 2023
@mergify mergify bot deleted the actionsetstatus-onphase branch January 3, 2023 20:50
Kanister automation moved this from Reviewer approved to Done Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants