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

backport: 1.4 backports 18-12-19 #6479

Merged
merged 33 commits into from Dec 20, 2018
Merged

backport: 1.4 backports 18-12-19 #6479

merged 33 commits into from Dec 20, 2018

Conversation

@ianvernon ianvernon added this to In Progress in 1.4 via automation Dec 19, 2018
@ianvernon ianvernon requested review from a team as code owners December 19, 2018 20:31
return "[]string"
}

func (m *MessageTypeFilter) Contains(typ int) bool {

Choose a reason for hiding this comment

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

exported method MessageTypeFilter.Contains should have comment or be unexported

return nil
}

func (m *MessageTypeFilter) Type() string {

Choose a reason for hiding this comment

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

exported method MessageTypeFilter.Type should have comment or be unexported

return strings.Join(pieces, ",")
}

func (m *MessageTypeFilter) Set(value string) error {

Choose a reason for hiding this comment

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

exported method MessageTypeFilter.Set should have comment or be unexported

MessageTypeAgent = 130
)

type MessageTypeFilter []int

Choose a reason for hiding this comment

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

exported type MessageTypeFilter should have comment or be unexported

tgraf and others added 25 commits December 19, 2018 12:36
[ upstream commit 6f2218f ]

The old --disable-ipv4 is marked as hidden but kept for backwards
compatibility. Map the DISABLE_IPV4 environment variable to --enable-ipv4 so
existing ConfigMap remain compatible.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 88ebfd7 ]

A flag already existed to indicate enablement but IPv6 was hardwired to
enabled.  Use the --enable-ipv6 value to control the node addressing API.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit e69d4db ]

This allows to disable IPv4 and IPv6 for docker networks. Only one of the two
address families have to be enabled.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit ec8973b ]

It only delays the setup process and adds no value. Docker is the source of
truth for docker endpoint IDs.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit e823b08 ]

Fail if neither IPv4 and IPv6 addressing is available

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 84f82e7 ]

Datapath:
Use a consistent ENABLE_IPV4 and ENABLE_IPV6 define to steer enabling IPv4 and
IPv6 code paths. Make all IPv6 datapath code dependent on ENABLE_IPV6.

Agent:
Make installation of routes, creation of maps, insertion into ipcache, etc.
dependent on option.Config.EnableIPv6.

Also fixes a problem in bpf/Makefile to properly test a combination of defines
when performing test compilation.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 6736b09 ]

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 196d083 ]

This ensures that memory resources are freed up when disabling an address
family.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 5104ff6 ]

The agent continues to enable IPv6 in the default setting but the default
ConfigMap will disable IPv6 for all new deployments. The motivation is
primarily a smaller memory footprint in default deployments.

This reduces the memory footprint in the default installation by about 200M.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 0cfc70c ]

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 85b03d4 ]

Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit f8ba9da ]

Replace the error creation in Map functions like Open(),OpenOrCreate()
to use os.PathError{}, which allows standard os.IsNotExist() to be used
to determine whether the map path exists or not when attempting to open
the map. It also combines the original error with the path and the
context, which is all our custom error types are trying to do anyway.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 4ed71e0 ]

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 69ce59d ]

It used to be that during startup, before CT maps are created by loading
a BPF program, the logs would be littered with warning messages about
how the CT maps don't exist, so garbage collection for CT cannot be
performed.

This commit checks the error type when opening the map, and if the path
doesn't exist, logs a message at info level rather than at warning
level. It's unlikely for the info-level message to be triggered after
the very initial startup of Cilium in a new environment, but in that
case the message will not be as concerning as a warning. If, however,
there really is some other type of error (for example, due to lack of
privileges to open the map type), then this should always be presented
as a warning.

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 6395ec3 ]

pkg/fqdn/dnsproxy.ExtractMsgDetails selected the lowest TTL incorrectly
when generating summary data about a DNS message. It would always choose
the uint32 maximum, when it should select the lowest TTL seen in answer
RRs.

Signed-off-by: Ray Bejjani <ray@covalent.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 4b72328 ]

`cilium identity get` only returns the identities which fully match the set of
labels provided to its `--labels` parameter. So, if an identity contains labels
'id=app1', and 'app=test', and `cilium identity get --labels=id=app1` is made,
then no identities will be returned because the label 'app=test' was not
provided to the command. This means that if an identity in the policy tracing
test overlaps with the labels being used in the trace, is still in the key-value
store, then the test will fail if any other prior test used the same labels
being used in the trace if `cilium identity get` is used to obtain all
identities with a single label, but there are other identities which contain
that given label and other labels.

Instead, to get all identities which contain the label used in the trace, use
\`cilium identity list\` and filter using \`jq\`.

Fixes: #6347.

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 1eed04b ]

The doc build is currently broken because of this dependency:
```
Exception occurred:
  File "/home/vagrant/.local/lib/python2.7/site-packages/versionwarning/extension.py", line 5, in <module>
    from .signals import generate_versionwarning_data_json
  File "/home/vagrant/.local/lib/python2.7/site-packages/versionwarning/signals.py", line 41
    **{config.versionwarning_message_placeholder: '<a href="#"></a>'},
```

Related: #6460

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 586003d ]

This limits the size of the idpool and saves memory.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 4012ee6 ]

There is only a few leased IDs at a time so the map can be allocated on demand.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 4868cae ]

Users of pkg/monitor are not necessiarly using the dissection functionality.
Initialize the parser on demand. This doesn't fully solve the memory
consumption problem as gopacket allocates a lot of memory into the RSS on its
own.

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit fdf644b ]

The existing default of 0.5M entries for the buffered go channel wastes a lot
of of memory in smaller environments. Reduce it to 32K and allow to customize
it for bigger environments via --monitor-queue-size=INT

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit aacdac3 ]

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 3fa6e02 ]

We tracked the exact IPs we inserted into rules previously, in addition
to those in the DNS cache, in order to avoid extra regenerations. This
is unnecessary and, ironically, caused more regenerations than intended.
This was because the bookeeping on previousEmittedIPs would delete the
entry as part of a rule update, which we did on each rule generation.
The code is now less implicitly circular in this regard.

Signed-off-by: Ray Bejjani <ray@covalent.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 888669d ]

Failing the agent can make it complicated to get everything up and running as
exponential backoff can cause cilium to not be running for a while which can
delay cilium-etcd from bootstrapping. This makes troubleshooting more
difficult.

Thus increase the timeout to 15 minutes to create a much larger overlap while
Cilium is up and running and etcd-operator can bring up a cluster in time.

Fixes: #6457

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 3a7d6f5 ]

Pinning is necessary for this map, as userspace needs to access it. Without this
option, the data path load of the map will not attempt to share the map with the
version on the filesystem.

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
Ian Vernon added 8 commits December 19, 2018 12:37
[ upstream commit 4f5757c ]

These fields represent the state which has not been plumbed to the datapath,
and the state which has been plumbed to the datapath related to policy.

Remove the L3Policy, L4Policy, and fields representing ingress and egress
policy enforcement from the Endpoint structure now that the desired / realized
policy contain all of these fields grouped together.

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 1f635b0 ]

The function has an L4Filter receiver, which makes this parameter useless, and
the function prone to misuse. Remove the paramter accordingly.

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 1a040bd ]

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 1511005 ]

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit 16c3461 ]

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
[ upstream commit e2d8a0b ]

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
…cture

[ upstream commit 14b329d ]

These are the identities which are denied based off of {From,To}Requires rules.
Compute these denied identity sets while policy is calculated for endpoint and
cache it.

The proxy requires knowledge of which identities are explicitly denied in order
to correctly implement policy rules imported by users. Previously, this was done
by performing another iteration over the rules to determine whether requirements
were not met at L3. This is an unneeded iteration over the rules, because the
iteration is already performed when policy is being computed for the endpoint.
When policy is calculated for the endpoint, cache the denied identities at
ingress and egress for the endpoint inside the Policy structure to avoid doing
unneeded iterations over the rules.

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
…il PolicyMap entry generation

[ upstream commit 872e102 ]

Due to now the L4Filter type is implemented, iterating over requirements
(ToRequires, FromRequires) is costly because it incurs a lot of DeepCopy and
matching operations on label selectors. Given that the set of denied identities
are now calculated in each direction, and cached in the new Policy structure, we
can skip this costly matching in the generation of the L4Filter. To do this we
can take advantage of the fact that while transforming the L4Filter into the set
of PolicyMap entries to which the L4Filter corresponds, we iterate over the set
of all known identities at the time of policy computation to translate from the
EndpointSelector-based fields in L4Filter to numicer identities. We can check
whether the identity being iterated over is in the set of denied identities at
L3 for that particular traffic direction, which is much cheaper in comparison to
DeepCopy and matching on EndpointSelectors.

To not break tracing, a flag is added into the SearchContext type, which skips
requirements matching if enabled. This is disabled by default, and is only
enabled when policy computation is performed within the ResolvePolicy function.

Signed-off by: Ian Vernon <ian@cilium.io>

Signed-off-by: Ian Vernon <ian@cilium.io>
@ianvernon
Copy link
Member Author

test-me-please

@ianvernon ianvernon closed this Dec 19, 2018
1.4 automation moved this from In Progress to Done Dec 19, 2018
@ianvernon ianvernon reopened this Dec 19, 2018
1.4 automation moved this from Done to In Progress Dec 19, 2018
@ianvernon
Copy link
Member Author

test-me-please

@ianvernon
Copy link
Member Author

test-docs-please

@ianvernon
Copy link
Member Author

test-missed-k8s

@ianvernon
Copy link
Member Author

@ianvernon
Copy link
Member Author

test-me-please

@ianvernon
Copy link
Member Author

Normal CI job got stuck building container image:
https://jenkins.cilium.io/job/Cilium-PR-Ginkgo-Tests-Validated/8615/

@ianvernon
Copy link
Member Author

test-me-please

@tgraf tgraf merged commit 1228e19 into v1.4 Dec 20, 2018
1.4 automation moved this from In Progress to Done Dec 20, 2018
@tgraf tgraf deleted the 1.4-backports-18-12-19 branch December 20, 2018 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
1.4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants