Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

fleetctl: Add restart and ssh-port functionality #961

Closed
wants to merge 1 commit into from

Conversation

ryandub
Copy link
Contributor

@ryandub ryandub commented Oct 9, 2014

Starts and stops a unit in one command.
Adds support to specify SSH port for SSH based commands:

  • journal
  • status
  • ssh
  • restart

refs #760
refs #932

@ryandub
Copy link
Contributor Author

ryandub commented Oct 9, 2014

Thanks to everyone's suggestions, I rewrote this to use SSH and systemctl. Because it is now using SSH, I needed to modify fleetctl's ssh handling to include custom ports - otherwise this will not work in my environments.

It works well and restarts units; however, one snag is that I am having trouble confirming that a unit has restarted successfully or not. systemctl restart <unit> returns 0 as long as the restart command is executed properly - even if the service will ultimately fail 2 seconds later. I tried adding a poller to check systemctl is-failed <unit> afterwards, but this is tricky because until the service fails, it shows as active and the amount of time it may take to run through ExecStartPre conditions can be variable, especially if those lines are being used to pull Docker images. Any suggestions on how to do this would be appreciated.

@@ -95,6 +100,7 @@ func runSSH(args []string) (exit int) {
sshClient, err = ssh.NewSSHClient("core", addr, getChecker(), flagSSHAgentForwarding)
}
if err != nil {
stderr("Failed building SSH client: %s", addr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add this information to the log line below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did not intend to leave that there. Will fix.

@bcwaldon
Copy link
Contributor

bcwaldon commented Oct 9, 2014

This isn't going to work with global units. It should probably just fail if a global unit is provided as an argument. @jonboulle ?

@ryandub
Copy link
Contributor Author

ryandub commented Oct 9, 2014

Thanks @bcwaldon - Fixed the unnecessary log line and changed the naming convention based on standard.

Starts and stops a unit in one command.
Adds support to specify SSH port for SSH based commands:
* journal
* status
* ssh
* restart

refs coreos#760
@bcwaldon
Copy link
Contributor

@ryandub Ok, only took me a month to get back to this :(

We've got a pretty big naming problem here. We currently support fleetctl start and fleetctl stop, which are not equivalent to systemctl start and systemctl stop. They simply communicate to fleet the desired state of the given units, and in the case of fleetctl start cause the units to be scheduled and loaded to a host. Alternatively, the systemctl commands require the load step to have already happened, then by default, synchronously start or stop a given unit.

Now in this PR, we're considering adding a fleetctl restart command, which is directly analogous to systemctl restart. I'm worried that this discrepancy in fleetctl start/stop & this new restart command are going to cause major confusion for people.

The only way I can see to resolve this is to rename the fleetctl start command to something like launch, and to make fleetctl start basically act as an RPC command to the host to-which a given unit was scheduled, basically like we do here with fleetctl restart. What do you think about this?

/cc @jonboulle

@bcwaldon
Copy link
Contributor

Filed #1025 to track the command inconsistency problem.

@ryandub Would you be able to separate out the --ssh-port piece into a separate PR so we can land that on its own?

@ryandub
Copy link
Contributor Author

ryandub commented Nov 26, 2014

@bcwaldon: just noticed these updates - sorry for the delay.

I'm happy to move the --ssh-port stuff to a separate PR. I can probably get that in next week. As for the restart naming, I agree - there is certainly some confusion and the proposals in #1025 make sense.

@bcwaldon
Copy link
Contributor

@ryandub Were you able to break apart the --ssh-port PR?

@bcwaldon
Copy link
Contributor

bcwaldon commented Jan 7, 2015

Please reopen if you're able to come back to this.

@anpieber
Copy link

anpieber commented Apr 7, 2015

Especially the fleetctl restart feature would be invaluable for continuous deployment into coreos (I just want to restart the service, triggering a new download and a restart, but don't know which machine the service is currently located).

@bcwaldon
Copy link
Contributor

bcwaldon commented Apr 7, 2015

@xueshanf This code did not land in master, the PR was just closed. However, #1148 just landed which covers the ssh-port part of this PR. That is available in master today.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants