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

feat: auto-create 1.x DB or 2.x bucket for flux task logs #2622

Merged
merged 2 commits into from
Sep 16, 2021
Merged

Conversation

lesam
Copy link
Contributor

@lesam lesam commented Sep 14, 2021

Closes #2620

Flux task logs are stored in the "Analytic Store", which is backed
by an influxdb 1.x Database or 2.x Bucket.

Auto-creation gives an infinite retention policy, which can be adjusted later by the user.

Note that for 1.x, "database/rp" bucket names will not be autocreated, only
bare "database" bucket names. This could potentially be added in the future
but the assumption is if a user wants control over the retention policy name
they probably want to control the retention policy creation as well.

Tested manually against 1.x and 2.x.

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

Closes #2620

Flux task logs are stored in the "Analytic Store", which is backed
by an influxdb 1.x Database or 2.x Bucket.

Auto-creation gives an infinite retention policy, which can be adjusted later by the user.

Note that for 1.x, "database/rp" bucket names will not be autocreated, only
bare "database" bucket names. This could potentially be added in the future
but the assumption is if a user wants control over the retention policy name
they probably want to control the retention policy creation as well.

Tested manually against 1.x and 2.x.
Comment on lines 35 to 57
for try := 0; try < numTries; try++ {
// poll for influxdb to come up - usually it should be already up by the time we get here.
if try > 0 {
time.Sleep(1 * time.Second)
}
queryForError := func(q string) error {
// This query is only successful if the bucket exists
result, err := cli.QueryFlux(influxdb.FluxQuery{
Org: dest.Org,
OrgID: dest.OrgID,
Query: q,
})
if err != nil {
return err
}
for result.More() {
if err := result.Next().Tables().Do(func(table flux.Table) error {
return table.Do(func(col flux.ColReader) error {
return nil
})
}); err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

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

If I'm reading this loop correctly, if we only succeed in creating the bucket on the last attempt, we will actually return an error even if the bucket was created. The bucket check and no error return is done at the top of the loop, and on the last iteration we can't get back to the top to check for success. Doesn't seem likely to happen in the real life, but might be confusing when it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -563,6 +563,11 @@ func (c *influxdbCluster) Open() error {

c.watchSubs()

// Even if we're not linking subscriptions, validate the client to ensure influxdb is actually up
if err := c.validateClientWithBackoff(ctx); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note this pings the configured influxdb and happens before we try to set up the flux task service.

Copy link
Member

@gwossum gwossum left a comment

Choose a reason for hiding this comment

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

Approved

@lesam lesam merged commit b498fce into master Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support auto-creation of InfluxDB database for flux task logs from kapacitor
2 participants