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

Stop recording update-status runs. #8132

Merged
merged 1 commit into from Nov 27, 2017

Conversation

howbazaar
Copy link
Contributor

The key purpose of the show-status-history command is to show historical events for a unit.

For any long running model, most units just show 'running update-status hook' followed by 'idle'. This isn't overly useful. So, now we don't record the 'running update-status hook' when the uniter runs it. The logging is still there, so it shows the hook running. When there is an error, this is still shown to the user.

For the workload status, if this value doesn't change, new history entries are no longer added. The current workload status is updated to reflect the timestamp change, but this history isn't added.

QA steps

juju deploy ./acceptancetests/repository/charms/update-status-tester tester
# wait 15 minutes or so so multiple update-status calls are made
juju show-status-history tester/0

During the testing for this branch, I found that show-status-history itself is a little broken in its display logic, so can check the database for the history values:

# agent status
db.statuseshistory.find({globalkey: "u#tester/0#charm"}).sort({updated:-1}).pretty()
# workload status
db.statuseshistory.find({globalkey: "u#tester/0"}).sort({updated:-1}).pretty()

Documentation changes

We should document that the update-status hook is no longer shown in juju status or the status history for a unit.

Bug reference

https://bugs.launchpad.net/juju/+bug/1530840

@howbazaar
Copy link
Contributor Author

Copy link
Member

@jameinel jameinel left a comment

Choose a reason for hiding this comment

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

One small tweak. The general behavior of omitting duplicates and then not going into Executing seems ok to me.

dataSame(current.StatusData, doc.StatusData) {
return
}
}
Copy link
Member

Choose a reason for hiding this comment

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

should we have an
} else if !mgo.NotFound(err) {
return err
}

I'm concerned that some other types of errors like "mongo died" would get swallowed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no error response from this method. It is a "maybeUpdate".

@howbazaar
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Nov 27, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

@jujubot jujubot merged commit e4f81c1 into juju:develop Nov 27, 2017
@howbazaar howbazaar deleted the skip-update-status-recording branch November 27, 2017 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants