mirror of
https://github.com/fluxcd/flagger.git
synced 2026-09-05 02:47:17 +00:00
Pujitha Paladugu
cf6cb64421
Motivation:
When canary.spec.service.portDiscovery is enabled, Flagger scans the
target Deployment/DaemonSet containers and adds any extra container
ports to the generated canary/primary/apex Services, so multi-port
apps stay reachable. The exclusion list used to skip mesh sidecars
only knew about Istio's container names ("istio-proxy", "envoy").
Linkerd's proxy injector uses the container name "linkerd-proxy", so
its internal ports (e.g. 4143 inbound, 4191 admin) were treated as
application ports and added to the generated Services instead of
being skipped. This was reported as a comment on #1345: "portDiscovery
does not work for linkerd" (using portDiscovery on a linkerd-meshed
deployment).
Note this does not address the original report in #1345, which does
not enable portDiscovery at all — dropping ports that only exist on a
pre-existing Service (and are not declared as container ports) is a
separate, broader problem that needs its own design discussion.
Approach:
Add "linkerd-proxy" to the sidecars exclusion map in
pkg/canary/util.go, matching how "istio-proxy" and "envoy" are
already excluded from getPorts().
Validation:
Added TestGetPortsExcludesSidecars to pkg/canary/util_test.go,
asserting that getPorts() drops linkerd-proxy and istio-proxy
container ports while keeping a regular app container's extra port.
go build ./...
go test ./pkg/canary/... -run TestGetPortsExcludesSidecars -v
Both passed. Also ran the full pkg/canary test suite (go test
./pkg/canary/...) with no regressions.
Fixes #1345
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
125 lines
2.9 KiB
Go
125 lines
2.9 KiB
Go
/*
|
|
Copyright 2020 The Flux authors
|
|
|
|
Licensed under the Apache License, Version 2.0 (the "License");
|
|
you may not use this file except in compliance with the License.
|
|
You may obtain a copy of the License at
|
|
|
|
http://www.apache.org/licenses/LICENSE-2.0
|
|
|
|
Unless required by applicable law or agreed to in writing, software
|
|
distributed under the License is distributed on an "AS IS" BASIS,
|
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
See the License for the specific language governing permissions and
|
|
limitations under the License.
|
|
*/
|
|
|
|
package canary
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
corev1 "k8s.io/api/core/v1"
|
|
|
|
flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
|
|
)
|
|
|
|
func TestIncludeLabelsByPrefix(t *testing.T) {
|
|
labels := map[string]string{
|
|
"foo": "foo-value",
|
|
"bar": "bar-value",
|
|
"lorem": "ipsum",
|
|
}
|
|
includeLabelPrefix := []string{"foo", "lor"}
|
|
|
|
filteredLabels := includeLabelsByPrefix(labels, includeLabelPrefix)
|
|
|
|
assert.Equal(t, filteredLabels, map[string]string{
|
|
"foo": "foo-value",
|
|
"lorem": "ipsum",
|
|
// bar excluded
|
|
})
|
|
}
|
|
|
|
func TestIncludeLabelsByPrefixWithWildcard(t *testing.T) {
|
|
labels := map[string]string{
|
|
"foo": "foo-value",
|
|
"bar": "bar-value",
|
|
"lorem": "ipsum",
|
|
}
|
|
includeLabelPrefix := []string{"*"}
|
|
|
|
filteredLabels := includeLabelsByPrefix(labels, includeLabelPrefix)
|
|
|
|
assert.Equal(t, filteredLabels, map[string]string{
|
|
"foo": "foo-value",
|
|
"bar": "bar-value",
|
|
"lorem": "ipsum",
|
|
})
|
|
}
|
|
|
|
func TestIncludeLabelsNoIncludes(t *testing.T) {
|
|
labels := map[string]string{
|
|
"foo": "foo-value",
|
|
"bar": "bar-value",
|
|
"lorem": "ipsum",
|
|
}
|
|
includeLabelPrefix := []string{""}
|
|
|
|
filteredLabels := includeLabelsByPrefix(labels, includeLabelPrefix)
|
|
|
|
assert.Equal(t, map[string]string{}, filteredLabels)
|
|
}
|
|
|
|
func TestGetPortsExcludesSidecars(t *testing.T) {
|
|
cd := &flaggerv1.Canary{
|
|
Spec: flaggerv1.CanarySpec{
|
|
Service: flaggerv1.CanaryService{
|
|
Port: 8080,
|
|
},
|
|
},
|
|
}
|
|
|
|
containers := []corev1.Container{
|
|
{
|
|
Name: "app",
|
|
Ports: []corev1.ContainerPort{
|
|
{Name: "http", ContainerPort: 8080},
|
|
{Name: "metrics", ContainerPort: 9090},
|
|
},
|
|
},
|
|
{
|
|
Name: "linkerd-proxy",
|
|
Ports: []corev1.ContainerPort{
|
|
{Name: "linkerd-proxy", ContainerPort: 4143},
|
|
{Name: "linkerd-admin", ContainerPort: 4191},
|
|
},
|
|
},
|
|
{
|
|
Name: "istio-proxy",
|
|
Ports: []corev1.ContainerPort{
|
|
{Name: "istio-proxy", ContainerPort: 15090},
|
|
},
|
|
},
|
|
}
|
|
|
|
ports := getPorts(cd, containers)
|
|
|
|
assert.Equal(t, map[string]int32{"metrics": 9090}, ports)
|
|
}
|
|
|
|
func TestMakePrimaryLabels(t *testing.T) {
|
|
labels := map[string]string{
|
|
"lorem": "ipsum",
|
|
"foo": "old-bar",
|
|
}
|
|
|
|
primaryLabels := makePrimaryLabels(labels, "new-bar", "foo")
|
|
|
|
assert.Equal(t, primaryLabels, map[string]string{
|
|
"lorem": "ipsum", // values from old map
|
|
"foo": "new-bar", // overriden value for a specific label
|
|
})
|
|
}
|