allow setting envoy proxy resource when using "expose" stanza

[fredwangwang]

Nomad version

v1.0.1

Operating system and Environment details

Linux

Issue

The default resource usage for envoy_sidecar is a bit too high for us, so we relying on sidecar_task to lower the resource:

sidecar_task {
    resources {
      cpu    = ...
      memory = ...
    }
  }

However, we found out once this is set, we are not able to set expose =true anymore:

So for now we have to remove the resource override for the envoy sidecar. But it really should be able to have expose and resource setting for sidecar at the same time.

Related line here:

nomad/nomad/job_endpoint_hook_expose_check.go

Lines 158 to 179 in 9a36ebe

	// serviceUsesConnectEnvoy returns true if the service is going to end up using
	// the built-in envoy proxy.
	//
	// This implementation is kind of reading tea leaves - firstly Connect
	// must be enabled, and second the sidecar_task must not be overridden. If these
	// conditions are met, the preceding connect hook will have injected a Connect
	// sidecar task, the configuration of which is interpolated at runtime.
	func serviceUsesConnectEnvoy(s *structs.Service) bool {
	// A non-nil connect stanza implies this service isn't connect enabled in
	// the first place.
	if s.Connect == nil {
	return false
	}
	// A non-nil connect.sidecar_task stanza implies the sidecar task is being
	// overridden (i.e. the default Envoy is not being used).
	if s.Connect.SidecarTask != nil {
	return false
	}
	return true
	}

[idrennanvmware]

Just touching base here. We are getting crushed in deployment resource requirements with mesh enabled and this feature on (expose). Just deployment of a single service with 4 instances is taking 1000mhz of cpu

This is one of those things that get exponentially worse with every service that goes to the mesh

[evandam]

Agreed this seems really important to be able to set. Alternatively, is there any way that the default resources can be set in the Nomad/Consul config so each job wouldn't have to set this?

[idrennanvmware]

@evandam

In our case we have different services with different levels of requests per second, we would want each service having it's own settings (and we encourage our teams to justify their resource usage) - but I can see the usefulness of a way to override the defaults at the global level.

[idrennanvmware]

I need to take a look deeper in to the code, but a thought struck me. Can it not be checked if the sidecar task is using the container specified in meta.connect.sidecar_image as part of the validation? If that value matches the image then overrides are "safe" at the resource level

[shoenig]

I think we can just remove the precondition altogether; this was originally added when we still had hope of Connect potentially being backed by more than just Envoy, but that seems unlikely now. expose is a leaky abstraction of an Envoy feature, which is why we were trying to be careful here.

I'll work on this today so we can slip the fix into 1.0.4

[idrennanvmware]

Thank you! This is a huge improvement for us!

[github-actions]

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.