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

backports to 1.1 2018-08-28 #5405

Closed
wants to merge 10 commits into from
Closed

Conversation

nebril
Copy link
Member

@nebril nebril commented Aug 28, 2018

#5300
#5284
#5252
#5250
#5230
#5206


This change is Reviewable

arvindsoni80 and others added 10 commits August 28, 2018 14:28
[ upstream commit b88691f ]

Prometheus and grafana are commonly deployed in the monitoring ns
the change makes it easy to connect those services without
requiring a full domain name for service

Signed-off-by: Arvind Soni <arvindsoni@gmail.com>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit a8e1512 ]

Include the map name here, so that the caller doesn't need to.

Related: cilium#5089

Signed-off-by: Joe Stringer <joe@covalent.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit a0fbf0a ]

Previously, the `errors` variable that would be returned was always
initialized to a non-nil empty slice, which would ensure that if the
caller checked it against nil that it would appear to always fail,
leading to the following false positive warning message:

unable to delete element 7439 from map /sys/fs/bpf/tc/globals/cilium_policy_7439: []

Fixes: cilium#5089

Signed-off-by: Joe Stringer <joe@covalent.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
…g endpoints

[ upstream commit a6d3a5d ]

Fixes: cilium#5290

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit fce1786 ]

This reverts commit c162b90.

Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit 50f29e9 ]

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit dd19c96 ]

Signed-off-by: Maciej Kwiek <maciej@covalent.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit bd42cec ]

Fixes: cilium#5317

Signed-off-by: Romain Lenglet <romain@covalent.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit d8314c7 ]

Create new listeners and remove obsolete listeners as early as
possible, just after updating the policy map. This gives more
time to Envoy to configure and ACK listener config updates,
and reduces the probability of regeneration timeouts due to
Envoy.

Also remove one endpoint mutex locking section after BPF
compilation, in order to reduce the regeneration time.

Signed-off-by: Romain Lenglet <romain@covalent.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
[ upstream commit 460c376 ]

The Go docs specify that a time.Duration "represents the elapsed
time between two instants as an int64 nanosecond count."
However, TimeoutConfig's Ticker and Timeout fields violated that
semantics as they were defined as time.Duration while storing
durations in Seconds.

Fix the timeout handling in WaitForServiceEndpoints, which
converted timeouts into billions of seconds.

To prevent such bugs from occurring again, specify clearly that
Ticker and Timeout contain seconds, and change their type to
int64.

Signed-off-by: Romain Lenglet <romain@covalent.io>
Signed-off-by: Maciej Kwiek <maciej.iai@gmail.com>
@nebril
Copy link
Member Author

nebril commented Aug 28, 2018

test-me-please

@jrfastab
Copy link
Contributor

test-me-please

@@ -712,6 +712,20 @@ func (e *Endpoint) regenerateBPF(owner Owner, epdir, reason string) (uint64, boo
}
}

// Walk the L4Policy to add new redirects and update the desired policy map
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes to this file look suspicious. I would revert just those.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK I'll drop the patch then.
ea5ec53

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's the one. Thanks!

@jrfastab
Copy link
Contributor

New PR created here, #5443

@tgraf
Copy link
Member

tgraf commented Sep 5, 2018

Closing in favor of #5443

@tgraf tgraf closed this Sep 5, 2018
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.

None yet

7 participants