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

Add sensu_event type #920

Merged
merged 3 commits into from Jul 6, 2018
Merged

Add sensu_event type #920

merged 3 commits into from Jul 6, 2018

Conversation

treydock
Copy link
Collaborator

Pull Request Checklist

Description

Related Issue

Part of #901 .

Motivation and Context

Allow sensu-go events to be resolved and deleted.

How Has This Been Tested?

Beaker acceptance tests and vagrant instance. Travis-ci tests will fail until #916 is merged.

General

  • Tests pass - bundle exec rake validate lint spec


def state
return @property_hash[:ensure]
=begin
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's this begin/end bit about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are block comments I forgot to remove after testing.

end

describe 'destroy' do
# before(:each) do
Copy link
Collaborator

Choose a reason for hiding this comment

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

why the commented out code in the commit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Commented out from file this was copied from, forgot to cleanup, will remove.

end
end

=begin
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Block comment, I'll remove.

].each do |property|
it "should not accept invalid #{property}" do
config[property] = 'foo bar'
expect { event }.to raise_error(Puppet::Error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably have an error message that is expected

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will update.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually this block is empty, so the test never actually gets executed. I left these empty test blocks around in case properties got added that fit the block's test criteria. There are several places in code these blocks are actually tested, I'll update those in separate pull request.

@ghoneycutt
Copy link
Collaborator

@treydock still getting errors after #916 was merged

@treydock treydock force-pushed the sensu2-event-type branch 3 times, most recently from c55241a to f9d0abb Compare June 25, 2018 22:53
@ghoneycutt
Copy link
Collaborator

ghoneycutt commented Jun 26, 2018

Having issues with acceptance tests, so canceling the build since it is long lived.

@treydock
Copy link
Collaborator Author

I thought that would be fixed by latest commit but it wasn't, will investigate. Thus far I haven't been able to reproduce locally using beaker and centos-7 set. The testing of events is rather tricky since events can't be created for the purpose of testing, at least not without first creating some checks and having them run and then forcing one to fail, but I fear all that effort would add a lot of time to the acceptance tests which some are already timing out.

@ghoneycutt
Copy link
Collaborator

Agreed that we should not do that extra effort which will lead to the tests timing out.

@treydock
Copy link
Collaborator Author

Issue resolved by forcing agent node to have specific entity ID and not relying on local hostname lookup as seems some containers kept getting localhost.localdomain and this collided with backend container.

@treydock treydock mentioned this pull request Jul 6, 2018
41 tasks
@ghoneycutt ghoneycutt merged commit 29726f7 into sensu:sensu2 Jul 6, 2018
@treydock treydock deleted the sensu2-event-type branch July 6, 2018 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants