Skip to content

Commit

Permalink
No need to expose http and grpc ports for a single check
Browse files Browse the repository at this point in the history
  • Loading branch information
ishustava committed May 10, 2021
1 parent b31aea2 commit 8efcfeb
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 157 deletions.
2 changes: 1 addition & 1 deletion .changelog/10173.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
agent: Save exposed Envoy ports to the agent's state when `Expose.Checks` is true in proxy's configuration.
```
```release-note:improvement
api: Add `ExposedHTTPPort` and `ExposedGRPCPort` to the health check API resource.
api: Add `ExposedPort` to the health check API resource.
```
```release-note:improvement
command: Exclude exposed Envoy ports from traffic redirection when providing `-proxy-id` and `Expose.Checks` is set.
Expand Down
12 changes: 6 additions & 6 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -2555,7 +2555,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
return err
}
http.ProxyHTTP = httpInjectAddr(http.HTTP, proxy.Address, port)
check.ExposedHTTPPort = port
check.ExposedPort = port
}

http.Start()
Expand Down Expand Up @@ -2625,7 +2625,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
return err
}
grpc.ProxyGRPC = grpcInjectAddr(grpc.GRPC, proxy.Address, port)
check.ExposedGRPCPort = port
check.ExposedPort = port
}

grpc.Start()
Expand Down Expand Up @@ -3812,7 +3812,7 @@ func (a *Agent) rerouteExposedChecks(serviceID structs.ServiceID, proxyAddr stri
}
c.ProxyHTTP = httpInjectAddr(c.HTTP, proxyAddr, port)
hc := a.State.Check(cid)
hc.ExposedHTTPPort = port
hc.ExposedPort = port
}
for cid, c := range a.checkGRPCs {
if c.ServiceID != serviceID {
Expand All @@ -3824,7 +3824,7 @@ func (a *Agent) rerouteExposedChecks(serviceID structs.ServiceID, proxyAddr stri
}
c.ProxyGRPC = grpcInjectAddr(c.GRPC, proxyAddr, port)
hc := a.State.Check(cid)
hc.ExposedGRPCPort = port
hc.ExposedPort = port
}
return nil
}
Expand All @@ -3838,15 +3838,15 @@ func (a *Agent) resetExposedChecks(serviceID structs.ServiceID) {
if c.ServiceID == serviceID {
c.ProxyHTTP = ""
hc := a.State.Check(cid)
hc.ExposedHTTPPort = 0
hc.ExposedPort = 0
ids = append(ids, cid)
}
}
for cid, c := range a.checkGRPCs {
if c.ServiceID == serviceID {
c.ProxyGRPC = ""
hc := a.State.Check(cid)
hc.ExposedGRPCPort = 0
hc.ExposedPort = 0
ids = append(ids, cid)
}
}
Expand Down
12 changes: 6 additions & 6 deletions agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4330,7 +4330,7 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("http", nil))
require.Equal(r, hc.ExposedHTTPPort, 21500)
require.Equal(r, hc.ExposedPort, 21500)
})

retry.Run(t, func(r *retry.R) {
Expand All @@ -4343,7 +4343,7 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("grpc", nil))
require.Equal(r, hc.ExposedGRPCPort, 21501)
require.Equal(r, hc.ExposedPort, 21501)
})

// Re-register a proxy and disable exposing HTTP checks.
Expand Down Expand Up @@ -4377,7 +4377,7 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("http", nil))
require.Equal(r, hc.ExposedHTTPPort, 0)
require.Equal(r, hc.ExposedPort, 0)
})

retry.Run(t, func(r *retry.R) {
Expand All @@ -4389,7 +4389,7 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("grpc", nil))
require.Equal(r, hc.ExposedGRPCPort, 0)
require.Equal(r, hc.ExposedPort, 0)
})
}

Expand Down Expand Up @@ -4483,7 +4483,7 @@ func TestAgent_RerouteNewHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("http", nil))
require.Equal(r, hc.ExposedHTTPPort, 21500)
require.Equal(r, hc.ExposedPort, 21500)
})

retry.Run(t, func(r *retry.R) {
Expand All @@ -4495,7 +4495,7 @@ func TestAgent_RerouteNewHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("grpc", nil))
require.Equal(r, hc.ExposedGRPCPort, 21501)
require.Equal(r, hc.ExposedPort, 21501)
})
}

Expand Down
10 changes: 3 additions & 7 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1413,13 +1413,9 @@ type HealthCheck struct {
ServiceTags []string // optional service tags
Type string // Check type: http/ttl/tcp/etc

// ExposedHTTPPort is the port of the exposed Envoy listener representing the
// HTTP health check of the service.
ExposedHTTPPort int

// ExposedGRPCPort is the port of the exposed Envoy listener representing the
// GRPC health check of the service.
ExposedGRPCPort int
// ExposedPort is the port of the exposed Envoy listener representing the
// HTTP or GRPC health check of the service.
ExposedPort int

Definition HealthCheckDefinition `bexpr:"-"`

Expand Down
9 changes: 2 additions & 7 deletions agent/structs/structs_filtering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,15 +545,10 @@ var expectedFieldConfigHealthCheck bexpr.FieldConfigurations = bexpr.FieldConfig
SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual, bexpr.MatchIn, bexpr.MatchNotIn, bexpr.MatchMatches, bexpr.MatchNotMatches},
StructFieldName: "Type",
},
"ExposedHTTPPort": &bexpr.FieldConfiguration{
"ExposedPort": &bexpr.FieldConfiguration{
CoerceFn: bexpr.CoerceInt,
SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual},
StructFieldName: "ExposedHTTPPort",
},
"ExposedGRPCPort": &bexpr.FieldConfiguration{
CoerceFn: bexpr.CoerceInt,
SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual},
StructFieldName: "ExposedGRPCPort",
StructFieldName: "ExposedPort",
},
}

Expand Down
25 changes: 12 additions & 13 deletions api/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,18 @@ const (

// AgentCheck represents a check known to the agent
type AgentCheck struct {
Node string
CheckID string
Name string
Status string
Notes string
Output string
ServiceID string
ServiceName string
Type string
ExposedHTTPPort int
ExposedGRPCPort int
Definition HealthCheckDefinition
Namespace string `json:",omitempty"`
Node string
CheckID string
Name string
Status string
Notes string
Output string
ServiceID string
ServiceName string
Type string
ExposedPort int
Definition HealthCheckDefinition
Namespace string `json:",omitempty"`
}

// AgentWeights represent optional weights for a service
Expand Down
7 changes: 2 additions & 5 deletions command/connect/redirecttraffic/redirect_traffic.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,8 @@ func (c *cmd) generateConfigFromFlags() (iptables.Config, error) {
}

for _, check := range checks {
if check.ExposedHTTPPort != 0 {
cfg.ExcludeInboundPorts = append(cfg.ExcludeInboundPorts, strconv.Itoa(check.ExposedHTTPPort))
}
if check.ExposedGRPCPort != 0 {
cfg.ExcludeInboundPorts = append(cfg.ExcludeInboundPorts, strconv.Itoa(check.ExposedGRPCPort))
if check.ExposedPort != 0 {
cfg.ExcludeInboundPorts = append(cfg.ExcludeInboundPorts, strconv.Itoa(check.ExposedPort))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion command/connect/redirecttraffic/redirect_traffic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ func TestGenerateConfigFromFlags(t *testing.T) {

if c.expError == "" {
require.NoError(t, err)
require.Equal(t, c.expCfg, cfg)
require.EqualValues(t, c.expCfg, cfg)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), c.expError)
Expand Down
6 changes: 2 additions & 4 deletions proto/pbservice/healthcheck.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 8efcfeb

Please sign in to comment.