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

fix(SSHChannelReader): Read output completely before checking command exit. #260

Merged
merged 1 commit into from
Apr 2, 2014

Conversation

calavera
Copy link
Contributor

@calavera calavera commented Apr 2, 2014

Fixes #259

readSSHChannelOutput(channel.Stderr)

exitStatus := -1
switch exitError := exitErr.(type) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, this switch will disappear in the future if this patch is accepted:

https://codereview.appspot.com/81290043/

@bcwaldon
Copy link
Contributor

bcwaldon commented Apr 2, 2014

@calavera This still does not fix the problem. The issue is in runRemoteCommand [0]. The ssh session is being closed before readSSHChannel even has a chance to read its stdout buffer.

[0]

defer sshClient.Close()

@calavera
Copy link
Contributor Author

calavera commented Apr 2, 2014

oh, I didn't see that. I don't think that's necessary. Let me run some tests.

@calavera
Copy link
Contributor Author

calavera commented Apr 2, 2014

I think it's working as expected now.

This is the output in the server:

core@ip-10-197-133-18 ~ $ journalctl --no-pager -l -u subgun-http.1.service
-- Logs begin at Sun 2014-03-23 21:52:09 UTC, end at Wed 2014-04-02 02:22:11 UTC. --
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:8] Unknown section 'X-Fleet'. Ignoring.
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:9] Assignment outside of section. Ignoring.
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: Starting subgun...
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: Started subgun.
Apr 02 02:22:03 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:8] Unknown section 'X-Fleet'. Ignoring.
Apr 02 02:22:03 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:9] Assignment outside of section. Ignoring.
Apr 02 02:22:04 ip-10-197-133-18 docker[27826]: Unable to find image 'coreos/subgun' locally
Apr 02 02:22:04 ip-10-197-133-18 docker[27826]: Pulling repository coreos/subgun
core@ip-10-197-133-18 ~ $

and this is the output of fleetctl:

calavera@dcp ~/oss/fleet/foo_Test$ ../bin/fleetctl journal subgun-http.1.service
-- Logs begin at Sun 2014-03-23 21:52:09 UTC, end at Wed 2014-04-02 02:22:29 UTC. --
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:8] Unknown section 'X-Fleet'. Ignoring.
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:9] Assignment outside of section. Ignoring.
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: Starting subgun...
Apr 02 02:22:02 ip-10-197-133-18 systemd[1]: Started subgun.
Apr 02 02:22:03 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:8] Unknown section 'X-Fleet'. Ignoring.
Apr 02 02:22:03 ip-10-197-133-18 systemd[1]: [/run/systemd/system/subgun-http.1.service:9] Assignment outside of section. Ignoring.
Apr 02 02:22:04 ip-10-197-133-18 docker[27826]: Unable to find image 'coreos/subgun' locally
Apr 02 02:22:04 ip-10-197-133-18 docker[27826]: Pulling repository coreos/subgun

@calavera
Copy link
Contributor Author

calavera commented Apr 2, 2014

Definitely not blocking this issue, but I think we should put all the logic to run commands together somewhere. Right now there are three places where it's duplicated:

  1. fleetctl/journal.go
  2. fleetctl/ssh.go
  3. fleetctl/status.go

@bcwaldon
Copy link
Contributor

bcwaldon commented Apr 2, 2014

@calavera Yes, this patch is not functioning properly. I'm going to merge it and try to refactor the code to your point above ^. I'll add functional testing around these features, too.

bcwaldon added a commit that referenced this pull request Apr 2, 2014
fix(SSHChannelReader): Read output completely before checking command exit.
@bcwaldon bcwaldon merged commit cd96507 into coreos:master Apr 2, 2014
@calavera calavera deleted the fix_command_channel_reader_race branch April 2, 2014 17:43
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.

Race condition in fleetctl journal
2 participants