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

Added home, data, config paths #1373

Merged
merged 2 commits into from
Apr 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ https://github.com/elastic/beats/compare/v5.0.0-alpha1...master[Check the HEAD d

*Filebeat*

* Default location for the registry file was changed to be `data/registry` from the binary directory,
rather than `.filebeat` in the current working directory. This affects installations for zip/tar.gz/source,
the location for DEB and RPM packages stays the same. {pull}1373[1373]

*Winlogbeat*


Expand Down Expand Up @@ -59,6 +63,8 @@ https://github.com/elastic/beats/compare/v5.0.0-alpha1...master[Check the HEAD d

*Affecting all Beats*

* Configuration options and CLI flags for setting the home, data and config paths. {pull}1373[1373]

*Packetbeat*

*Topbeat*
Expand Down
2 changes: 1 addition & 1 deletion filebeat/.gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/etc/filebeat.dev.yml
.idea
.vagrant
.filebeat
/data/
/docs/html_docs

filebeat
Expand Down
2 changes: 1 addition & 1 deletion filebeat/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

// Defaults for config variables which are not set
const (
DefaultRegistryFile = ".filebeat"
DefaultRegistryFile = "registry"
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add the .json extension to the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I used registry is that on DEB/RPM installations the file name already used is /usr/share/filebeat/registry. By using registry we can keep BC for those cases at least.

In addition, JSON is an implementation detail which we might change in the future. Also, adding the extension could be inviting people to edit the file, which I guess is the last thing we want.

I see the three +1, but is there a strong reason to add the .json?

Copy link
Member

Choose a reason for hiding this comment

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

Removed my +1 as I think the point about implementation and potentially changing in the future is a good point. So even if we switch to the format "xyz" the file name stays the same.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong opinion here. So leaving it as registry is fine with me. (I never meant to +1 my own comment anyways; I didn't realize clicking the thumb icon added a +1 until after I clicked it, and then I couldn't figure out how to undo.)

The reason I used registry is that on DEB/RPM installations the file name already used is /usr/share/filebeat/registry. By using registry we can keep BC for those cases at least.

Those installations were using a non-default value so they should not be affected by the change.

In addition, JSON is an implementation detail which we might change in the future.

True, leaving it without an extension makes it easier to change underlying file 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.

Those installations were using a non-default value so they should not be affected by the change.

Good point. But if it's just registry I can get rid of one of those nasty sed commands on the configuration file at package time by just setting the data path in the init script.

DefaultIgnoreOlderDuration time.Duration = 0
DefaultCloseOlderDuration time.Duration = 1 * time.Hour
DefaultScanFrequency time.Duration = 10 * time.Second
Expand Down
14 changes: 5 additions & 9 deletions filebeat/crawler/registrar.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/elastic/beats/filebeat/input"
. "github.com/elastic/beats/filebeat/input"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/paths"
)

type Registrar struct {
Expand Down Expand Up @@ -46,17 +47,12 @@ func (r *Registrar) Init() error {
r.registryFile = cfg.DefaultRegistryFile
}

// Make sure the directory where we store the registryFile exists
absPath, err := filepath.Abs(r.registryFile)
if err != nil {
return fmt.Errorf("Failed to get the absolute path of %s: %v",
r.registryFile, err)
}
r.registryFile = absPath
// The registry file is opened in the data path
r.registryFile = paths.Resolve(paths.Data, r.registryFile)

// Create directory if it does not already exist.
registryPath := filepath.Dir(r.registryFile)
err = os.MkdirAll(registryPath, 0755)
err := os.MkdirAll(registryPath, 0755)
if err != nil {
return fmt.Errorf("Failed to created registry file dir %s: %v",
registryPath, err)
Expand All @@ -68,7 +64,7 @@ func (r *Registrar) Init() error {
}

// loadState fetches the previous reading state from the configure RegistryFile file
// The default file is .filebeat file which is stored in the same path as the binary is running
// The default file is `registry` in the data path.
func (r *Registrar) LoadState() {
if existing, e := os.Open(r.registryFile); e == nil {
defer existing.Close()
Expand Down
4 changes: 2 additions & 2 deletions filebeat/docs/migration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ The registry file stores the state and location information that Filebeat uses t
where it was last reading. Under Logstash Forwarder, this file was called `.logstash-fowarder`. For Filebeat,
the file was renamed. The name varies depending on the package type:

* `.filebeat` for `.tar.gz` and `.tgz` archives
* `data/registry` for `.tar.gz` and `.tgz` archives
* `/var/lib/filebeat/registry` for DEB and RPM packages
* `c:\ProgramData\filebeat\registry` for the Windows zip file

Expand Down Expand Up @@ -426,7 +426,7 @@ with the new packages.
One notable change is the name of the registry file. The name varies depending on the package
type:

* `.filebeat` for `.tar.gz` and `.tgz` archives
* `registry` for `.tar.gz` and `.tgz` archives
* `/usr/lib/filebeat/registry` for DEB and RPM packages
* `c:\ProgramData\filebeat\registry` for the Windows zip file

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,13 @@ filebeat:

===== registry_file

The name of the registry file. By default, the registry file is put in the current
working directory. If the working directory changes for subsequent runs of Filebeat, indexing starts from the beginning again.
The name of the registry file. If a relative path is used, it is considered relative to the
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what you mean by data path here. Do you mean relative to the folders that are being crawled? Does that mean there is more than one registry file?

data path. The default is `registry`.

[source,yaml]
-------------------------------------------------------------------------------------
filebeat:
registry_file: .filebeat
registry_file: registry
-------------------------------------------------------------------------------------


Expand Down
7 changes: 3 additions & 4 deletions filebeat/etc/beat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,9 @@ filebeat:
# Flush even though spool_size is not reached.
#idle_timeout: 5s

# Name of the registry file. Per default it is put in the current working
# directory. In case the working directory is changed after when running
# filebeat again, indexing starts from the beginning again.
#registry_file: .filebeat
# Name of the registry file. If a relative path is used, it is considered relative to the
# data path.
#registry_file: registry

# Full Path to directory with additional prospector configuration files. Each file must end with .yml
# These config files must have the full filebeat config part inside, but only
Expand Down
7 changes: 3 additions & 4 deletions filebeat/filebeat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,9 @@ filebeat:
# Flush even though spool_size is not reached.
#idle_timeout: 5s

# Name of the registry file. Per default it is put in the current working
# directory. In case the working directory is changed after when running
# filebeat again, indexing starts from the beginning again.
#registry_file: .filebeat
# Name of the registry file. If a relative path is used, it is considered relative to the
# data path.
#registry_file: registry

# Full Path to directory with additional prospector configuration files. Each file must end with .yml
# These config files must have the full filebeat config part inside, but only
Expand Down
32 changes: 16 additions & 16 deletions filebeat/input/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,50 +60,50 @@ func TestSafeFileRotateExistingFile(t *testing.T) {
assert.NoError(t, os.RemoveAll(tempdir))
}()

// create an existing .filebeat file
err = ioutil.WriteFile(filepath.Join(tempdir, ".filebeat"),
// create an existing registry file
err = ioutil.WriteFile(filepath.Join(tempdir, "registry"),
[]byte("existing filebeat"), 0x777)
assert.NoError(t, err)

// create a new .filebeat.new file
err = ioutil.WriteFile(filepath.Join(tempdir, ".filebeat.new"),
// create a new registry.new file
err = ioutil.WriteFile(filepath.Join(tempdir, "registry.new"),
[]byte("new filebeat"), 0x777)
assert.NoError(t, err)

// rotate .filebeat.new into .filebeat
err = SafeFileRotate(filepath.Join(tempdir, ".filebeat"),
filepath.Join(tempdir, ".filebeat.new"))
// rotate registry.new into registry
err = SafeFileRotate(filepath.Join(tempdir, "registry"),
filepath.Join(tempdir, "registry.new"))
assert.NoError(t, err)

contents, err := ioutil.ReadFile(filepath.Join(tempdir, ".filebeat"))
contents, err := ioutil.ReadFile(filepath.Join(tempdir, "registry"))
assert.NoError(t, err)
assert.Equal(t, []byte("new filebeat"), contents)

// do it again to make sure we deal with deleting the old file

err = ioutil.WriteFile(filepath.Join(tempdir, ".filebeat.new"),
err = ioutil.WriteFile(filepath.Join(tempdir, "registry.new"),
[]byte("new filebeat 1"), 0x777)
assert.NoError(t, err)

err = SafeFileRotate(filepath.Join(tempdir, ".filebeat"),
filepath.Join(tempdir, ".filebeat.new"))
err = SafeFileRotate(filepath.Join(tempdir, "registry"),
filepath.Join(tempdir, "registry.new"))
assert.NoError(t, err)

contents, err = ioutil.ReadFile(filepath.Join(tempdir, ".filebeat"))
contents, err = ioutil.ReadFile(filepath.Join(tempdir, "registry"))
assert.NoError(t, err)
assert.Equal(t, []byte("new filebeat 1"), contents)

// and again for good measure

err = ioutil.WriteFile(filepath.Join(tempdir, ".filebeat.new"),
err = ioutil.WriteFile(filepath.Join(tempdir, "registry.new"),
[]byte("new filebeat 2"), 0x777)
assert.NoError(t, err)

err = SafeFileRotate(filepath.Join(tempdir, ".filebeat"),
filepath.Join(tempdir, ".filebeat.new"))
err = SafeFileRotate(filepath.Join(tempdir, "registry"),
filepath.Join(tempdir, "registry.new"))
assert.NoError(t, err)

contents, err = ioutil.ReadFile(filepath.Join(tempdir, ".filebeat"))
contents, err = ioutil.ReadFile(filepath.Join(tempdir, "registry"))
assert.NoError(t, err)
assert.Equal(t, []byte("new filebeat 2"), contents)
}
Expand Down
2 changes: 1 addition & 1 deletion filebeat/tests/load/filebeat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ filebeat:

spool_size: 4096
idle_timeout: 5s
registry_file: .filebeat
registry_file: registry


############################# Output ##########################################
Expand Down
9 changes: 8 additions & 1 deletion filebeat/tests/system/config/filebeat.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ filebeat:
{% endif %}
spool_size:
idle_timeout: 0.1s
registry_file: {{ beat.working_dir + '/' }}{{ registryFile|default(".filebeat")}}
{% if not skip_registry_config %}
registry_file: {{ beat.working_dir + '/' }}{{ registryFile|default("registry")}}
{%endif%}


############################# Shipper ############################################
Expand Down Expand Up @@ -146,4 +148,9 @@ filter:
{%- endif %}
{% endif %}

{% if path_data %}
path:
data: {{path_data}}
{%endif%}

# vim: set ft=jinja:
2 changes: 1 addition & 1 deletion filebeat/tests/system/config/filebeat_prospectors.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ filebeat:
encoding: {{prospector.encoding | default("plain") }}
{% endfor %}
idle_timeout: 0.5s
registry_file: {{ beat.working_dir + '/' }}{{ registryFile|default(".filebeat")}}
registry_file: {{ beat.working_dir + '/' }}{{ registryFile|default("registry")}}

output:
file:
Expand Down
6 changes: 3 additions & 3 deletions filebeat/tests/system/filebeat.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ def setUpClass(self):
self.beat_name = "filebeat"
super(BaseTest, self).setUpClass()

def get_dot_filebeat(self):
# Returns content of the .filebeat file
dotFilebeat = self.working_dir + '/.filebeat'
def get_registry(self):
# Returns content of the registry file
dotFilebeat = self.working_dir + '/registry'
assert os.path.isfile(dotFilebeat) is True

with open(dotFilebeat) as file:
Expand Down
6 changes: 3 additions & 3 deletions filebeat/tests/system/test_crawler.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ def test_file_disappear(self):

filebeat.check_kill_and_wait()

data = self.get_dot_filebeat()
data = self.get_registry()

# Make sure new file was picked up, old file should stay in
assert len(data) == 2
Expand Down Expand Up @@ -315,7 +315,7 @@ def test_file_disappear_appear(self):

filebeat.check_kill_and_wait()

data = self.get_dot_filebeat()
data = self.get_registry()

# Make sure new file was picked up. As it has the same file name,
# only one entry exists
Expand Down Expand Up @@ -373,7 +373,7 @@ def test_force_close(self):

filebeat.check_kill_and_wait()

data = self.get_dot_filebeat()
data = self.get_registry()

# Make sure new file was picked up. As it has the same file name,
# only one entry exists
Expand Down
41 changes: 27 additions & 14 deletions filebeat/tests/system/test_registrar.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ def test_registrar_file_content(self):
# the logging and actual writing the file. Seems to happen on Windows.
self.wait_until(
lambda: os.path.isfile(os.path.join(self.working_dir,
".filebeat")),
"registry")),
max_timeout=1)
filebeat.check_kill_and_wait()

# Check that a single file exists in the registry.
data = self.get_dot_filebeat()
data = self.get_registry()
assert len(data) == 1

logFileAbsPath = os.path.abspath(testfile)
Expand Down Expand Up @@ -112,12 +112,12 @@ def test_registrar_files(self):
# the logging and actual writing the file. Seems to happen on Windows.
self.wait_until(
lambda: os.path.isfile(os.path.join(self.working_dir,
".filebeat")),
"registry")),
max_timeout=1)
filebeat.check_kill_and_wait()

# Check that file exist
data = self.get_dot_filebeat()
data = self.get_registry()

# Check that 2 files are port of the registrar file
assert len(data) == 2
Expand Down Expand Up @@ -156,7 +156,7 @@ def test_rotating_file(self):
Checks that the registry is properly updated after a file is rotated
"""
self.render_config_template(
path=os.path.abspath(self.working_dir) + "/log/*"
path=os.path.abspath(self.working_dir) + "/log/*"
)

os.mkdir(self.working_dir + "/log/")
Expand All @@ -167,30 +167,43 @@ def test_rotating_file(self):
with open(testfile, 'w') as f:
f.write("offset 9\n")

self.wait_until(
lambda: self.output_has(lines=1),
max_timeout=10)

self.wait_until(lambda: self.output_has(lines=1),
max_timeout=10)

testfilerenamed = self.working_dir + "/log/test.1.log"
os.rename(testfile, testfilerenamed)

with open(testfile, 'w') as f:
f.write("offset 10\n")


self.wait_until(
lambda: self.output_has(lines=2),
max_timeout=10)
self.wait_until(lambda: self.output_has(lines=2),
max_timeout=10)

filebeat.check_kill_and_wait()

# Check that file exist
data = self.get_dot_filebeat()
data = self.get_registry()

# Make sure the offsets are correctly set
data[os.path.abspath(testfile)]["offset"] = 10
data[os.path.abspath(testfilerenamed)]["offset"] = 9

# Check that 2 files are port of the registrar file
assert len(data) == 2

def test_data_path(self):
"""
Checks that the registry file is written in a custom data path.
"""
self.render_config_template(
path=self.working_dir + "/test.log",
path_data=self.working_dir + "/datapath",
skip_registry_config=True,
)
with open(self.working_dir + "/test.log", "w") as f:
f.write("test message\n")
filebeat = self.start_beat()
self.wait_until(lambda: self.output_has(lines=1))
filebeat.check_kill_and_wait()

assert os.path.isfile(self.working_dir + "/datapath/registry")
Loading