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 transient states ("stopping", "starting") #8730

Closed
kschaab opened this issue Jul 15, 2020 · 10 comments · Fixed by #8868
Closed

Add transient states ("stopping", "starting") #8730

kschaab opened this issue Jul 15, 2020 · 10 comments · Fixed by #8868
Assignees
Labels
july-chill kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. ux/embedded Embedded UX blockers

Comments

@kschaab
Copy link
Contributor

kschaab commented Jul 15, 2020

Minikube status lists Running, Paused or Stopped for each profile. It does not account for the state between minikube start and Running. It would be helpful to indicate when minikube is starting, pausing (maybe, this state seems like it might be super short), stopping and possibly deleting as well as valid states.

@medyagh medyagh added kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jul 15, 2020
@medyagh
Copy link
Member

medyagh commented Jul 15, 2020

seems like a good feature for minikube !

@tstromberg tstromberg changed the title Minikube should report transient status when transitioning between states Add transient states ("stopping", "starting") Jul 17, 2020
@tstromberg tstromberg added ux/embedded Embedded UX blockers july-chill and removed july-chill labels Jul 17, 2020
@tstromberg
Copy link
Contributor

"downloading" could also be a good state

@tharun208
Copy link
Contributor

I like to work on this @tstromberg

@medyagh
Copy link
Member

medyagh commented Jul 20, 2020

@tharun208 I would be happy to review a PR that doe this. do you think you can make it for v1.13.0 on Aug 4th?

@tstromberg
Copy link
Contributor

@tharun208 - please confirm if you would still like to take this on. Here is my initial naive implementation idea:

  • Create a new file (for example, ~/.minikube/profiles/<name>/state.json) containing a simple JSON file with a field like:

{ status = "starting" }

  • Add a Status function here that returns the path. You can use ConfigFile() as a reference to copy:

  • Add code to

    func Status(api libmachine.API, machineName string) (string, error) {
    that references the state output by this file. I'd probably start by ignoring this file if it's modification time is older than 1 minute.

  • Add a method in the same package that allows you to update the state file

  • Add code to

    func StartHost(api libmachine.API, cfg *config.ClusterConfig, n *config.Node) (*host.Host, bool, error) {
    to set the state to Starting

  • Add code to

    func StopHost(api libmachine.API, machineName string) error {
    to set the state to Stopping.

Other ideas, though I wouldn't necessarily worry about them for the initial implementation, are Downloading and Deleting.

@tstromberg
Copy link
Contributor

tstromberg commented Jul 21, 2020

If I haven't heard anything back later today from @tharun208, I'd like to work on this, as it's blocking one of our users (Cloud Code).

One new idea I had is that @priyawadhwa built the concept of "steps" into minikube, for emitting CloudEvent JSON output:

register.Reg.SetStep(register.CreatingContainer)

I think we should build on this by introducing a concept of state transitions, and storing the combined state transition & step into the status file. For instance:

state.Transition(state.Starting, register.CreatingContainer, message)

Could emit the appropriate output.

@priyawadhwa
Copy link

@tstromberg instead of passing in the specific step register.CreatingContainer, you could access the current step via:

register.Reg.CurrentStep()

that way you could rely on the steps that have already been set in the register instead of having to uniquely pass them in again

@tharun208
Copy link
Contributor

@medyagh, I think I can't make it for the next release. @tstromberg, you can pick this

@tstromberg tstromberg self-assigned this Jul 21, 2020
@tstromberg
Copy link
Contributor

Thanks for the heads up @tharun208. I'm sure this feature will require future refinement where we could use your hepl.

@tstromberg
Copy link
Contributor

tstromberg commented Jul 21, 2020

I have some ideas on this, and intend to have an initial PR for review tomorrow, with a target to merge by EOW.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
july-chill kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. ux/embedded Embedded UX blockers
Projects
None yet
5 participants