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.

Sign up for GitHub

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

Jump to bottom

Test sset list with pods not in sset #1936

Merged
merged 1 commit into from
Oct 9, 2019
Merged

Test sset list with pods not in sset #1936

merged 1 commit into from
Oct 9, 2019

Conversation

racevedoo
Copy link
Contributor

@racevedoo racevedoo commented Oct 8, 2019 •
edited
Loading

This PR is simple: it only removes a TODO that said we should test sset list with pods that do not belong to the sset. It was not done before due to kubernetes-sigs/controller-runtime#311. Relates to #1800

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@racevedoo racevedoo mentioned this pull request Oct 8, 2019
Open
sebgl
sebgl reviewed Oct 9, 2019
View reviewed changes
Copy link
Contributor

@sebgl sebgl left a comment

Choose a reason for hiding this comment

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

Thanks for fixing that @racevedoo! I left a tiny suggestion to make things more explicit. Otherwise LGTM.

pkg/controller/elasticsearch/sset/list_test.go Outdated Show resolved Hide resolved
@sebgl
Copy link
Contributor

sebgl commented Oct 9, 2019

Jenkins test this please.

@thbkrkr thbkrkr added the justdoit Continuous improvement not related to a specific feature label Oct 9, 2019
@racevedoo racevedoo requested a review from sebgl October 9, 2019 12:55
@pebrc pebrc added the >test Related to unit/integration/e2e tests label Oct 9, 2019
sebgl
sebgl approved these changes Oct 9, 2019
View reviewed changes
Copy link
Contributor

@sebgl sebgl left a comment

Choose a reason for hiding this comment

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

LGTM, let's wait for CI results before merging.

@thbkrkr
Copy link
Contributor

thbkrkr commented Oct 9, 2019

Jenkins test this please.

@racevedoo
Copy link
Contributor Author

racevedoo commented Oct 9, 2019 •
edited
Loading

Hey guys, I guess jenkins got a timeout. From console output:

15:42:20  go: finding sigs.k8s.io v0.2.1
15:42:20  go: finding sigs.k8s.io/controller-tools/cmd v0.2.1
15:42:20  go: finding sigs.k8s.io/controller-tools/cmd/controller-gen v0.2.1
15:42:47  verifying sigs.k8s.io/controller-tools@v0.2.1: sigs.k8s.io/controller-tools@v0.2.1: Get https://sum.golang.org/lookup/sigs.k8s.io/controller-tools@v0.2.1: dial tcp 108.177.120.141:443: i/o timeout

Can someone trigger another build please?

@sebgl
Copy link
Contributor

sebgl commented Oct 9, 2019

Jenkins test this please

@thbkrkr thbkrkr merged commit 4909bc5 into elastic:master Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebgl sebgl sebgl approved these changes

Assignees
No one assigned
Labels
justdoit Continuous improvement not related to a specific feature >test Related to unit/integration/e2e tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@racevedoo @elasticmachine @sebgl @thbkrkr @pebrc