Skip to content

Commit

Permalink
Merge pull request #3497 from telepresenceio/thallgren/helm-reuse-sem…
Browse files Browse the repository at this point in the history 
…antics

Ensure that telepresence upgrade uses helm semantics for reuse/reset.
  • Loading branch information
@thallgren
thallgren authored Jan 31, 2024
2 parents fa5608a + b6fabea commit 030848a
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 55 deletions.
56 changes: 28 additions & 28 deletions integration_test/inject_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,42 +11,42 @@ import (
"github.com/telepresenceio/telepresence/v2/pkg/agentconfig"
)

func (s *installSuite) applyPolicyApp(ctx context.Context, name, namespace string, wg *sync.WaitGroup) {
func (is *installSuite) applyPolicyApp(ctx context.Context, name, namespace string, wg *sync.WaitGroup) {
defer wg.Done()
s.T().Helper()
is.T().Helper()
manifest := filepath.Join("testdata", "k8s", name+".yaml")
s.NoError(itest.Kubectl(ctx, namespace, "apply", "-f", manifest), "failed to apply %s", manifest)
s.NoError(itest.RolloutStatusWait(ctx, namespace, "deploy/"+name))
is.NoError(itest.Kubectl(ctx, namespace, "apply", "-f", manifest), "failed to apply %s", manifest)
is.NoError(itest.RolloutStatusWait(ctx, namespace, "deploy/"+name))
}

func (s *installSuite) assertInjected(ctx context.Context, name, namespace string, present bool, wg *sync.WaitGroup) {
func (is *installSuite) assertInjected(ctx context.Context, name, namespace string, present bool, wg *sync.WaitGroup) {
defer wg.Done()
s.T().Helper()
is.T().Helper()
out, err := itest.KubectlOut(ctx, namespace, "get", "pods", "-l", "app="+name, "-o", "jsonpath={.items.*.spec.containers[?(@.name=='traffic-agent')].image}")
s.NoError(err)
is.NoError(err)
n := "tel2"
if ai := itest.GetAgentImage(ctx); ai != nil {
n = ai.Name
}
n = "/" + n + ":"
if present {
s.Contains(out, n)
is.Contains(out, n)
} else {
s.NotContains(out, n)
is.NotContains(out, n)
}
}

func (s *installSuite) injectPolicyTest(ctx context.Context, policy agentconfig.InjectPolicy) {
namespace := fmt.Sprintf("%s-%s", strings.ToLower(policy.String()), s.Suffix())
func (is *installSuite) injectPolicyTest(ctx context.Context, policy agentconfig.InjectPolicy) {
namespace := fmt.Sprintf("%s-%s", strings.ToLower(policy.String()), is.Suffix())
itest.CreateNamespaces(ctx, namespace)
defer itest.DeleteNamespaces(ctx, namespace)

ctx = itest.WithNamespaces(ctx, &itest.Namespaces{
Namespace: namespace,
ManagedNamespaces: []string{namespace},
})
s.NoError(s.TelepresenceHelmInstall(ctx, false, "--set", "agentInjector.injectPolicy="+policy.String()))
defer s.UninstallTrafficManager(ctx, namespace)
is.NoError(is.TelepresenceHelmInstall(ctx, false, "--set", "agentInjector.injectPolicy="+policy.String()))
defer is.UninstallTrafficManager(ctx, namespace)

ctx = itest.WithUser(ctx, namespace+":"+itest.TestUser)
itest.TelepresenceOk(ctx, "connect", "--namespace", namespace, "--manager-namespace", namespace)
Expand All @@ -56,46 +56,46 @@ func (s *installSuite) injectPolicyTest(ctx context.Context, policy agentconfig.

wg := sync.WaitGroup{}
wg.Add(3)
go s.applyPolicyApp(ctx, "pol-enabled", namespace, &wg)
go s.applyPolicyApp(ctx, "pol-none", namespace, &wg)
go s.applyPolicyApp(ctx, "pol-disabled", namespace, &wg)
go is.applyPolicyApp(ctx, "pol-enabled", namespace, &wg)
go is.applyPolicyApp(ctx, "pol-none", namespace, &wg)
go is.applyPolicyApp(ctx, "pol-disabled", namespace, &wg)
wg.Wait()
if s.T().Failed() {
if is.T().Failed() {
return
}

// No pod should have a traffic-agent at this stage, except for the pol-enabled when the policy is WhenEnabled
wg.Add(3)
go s.assertInjected(ctx, "pol-enabled", namespace, true, &wg) // always injected in advance
go s.assertInjected(ctx, "pol-none", namespace, false, &wg) // never injected in advance
go s.assertInjected(ctx, "pol-disabled", namespace, false, &wg) // never injected
go is.assertInjected(ctx, "pol-enabled", namespace, true, &wg) // always injected in advance
go is.assertInjected(ctx, "pol-none", namespace, false, &wg) // never injected in advance
go is.assertInjected(ctx, "pol-disabled", namespace, false, &wg) // never injected
wg.Wait()
if s.T().Failed() {
if is.T().Failed() {
return
}

// An intercept on the pol-disabled must always fail
wg.Add(1)
go func() {
_, _, err := itest.Telepresence(ctx, "intercept", "--mount", "false", "pol-disabled", "--", "true")
s.Error(err)
s.assertInjected(ctx, "pol-disabled", namespace, false, &wg)
is.Error(err)
is.assertInjected(ctx, "pol-disabled", namespace, false, &wg)
}()

// for OnDemand, an intercept on the pol-none must succeed inject the agent
if policy == agentconfig.OnDemand {
wg.Add(1)
_, _, err := itest.Telepresence(ctx, "intercept", "--mount", "false", "pol-none", "--", "true")
s.NoError(err)
s.assertInjected(ctx, "pol-none", namespace, true, &wg)
is.NoError(err)
is.assertInjected(ctx, "pol-none", namespace, true, &wg)
}
wg.Wait()
}

func (s *installSuite) TestInjectPolicy() {
func (is *installSuite) TestInjectPolicy() {
for _, policy := range []agentconfig.InjectPolicy{agentconfig.OnDemand, agentconfig.WhenEnabled} {
s.Run(policy.String(), func() {
s.injectPolicyTest(s.Context(), policy)
is.Run(policy.String(), func() {
is.injectPolicyTest(is.Context(), policy)
})
}
}
74 changes: 74 additions & 0 deletions integration_test/install_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import (
"strings"
"time"

"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/cli/values"
"helm.sh/helm/v3/pkg/getter"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/client-go/tools/clientcmd/api"

"github.com/datawire/dlib/dlog"
Expand Down Expand Up @@ -47,6 +49,78 @@ func init() {
})
}

func getHelmConfig(ctx context.Context, configFlags *genericclioptions.ConfigFlags, namespace string) (*action.Configuration, error) {
helmConfig := &action.Configuration{}
err := helmConfig.Init(configFlags, namespace, "secrets", func(format string, args ...any) {
ctx := dlog.WithField(ctx, "source", "helm")
dlog.Infof(ctx, format, args...)
})
if err != nil {
return nil, err
}
return helmConfig, nil
}

func (is *installSuite) Test_UpgradeRetainsValues() {
ctx := is.Context()
rq := is.Require()
rq.NoError(is.TelepresenceHelmInstall(ctx, false, "--set", "logLevel=debug"))
defer is.UninstallTrafficManager(ctx, is.ManagerNamespace())

ctx, kc := is.cluster(ctx, "", is.ManagerNamespace())
helmConfig, err := getHelmConfig(ctx, kc.ConfigFlags, is.ManagerNamespace())
rq.NoError(err)

getValues := func() (map[string]any, error) {
return action.NewGetValues(helmConfig).Run("traffic-manager")
}
containsKey := func(m map[string]any, key string) bool {
_, ok := m[key]
return ok
}

oldValues, err := getValues()
rq.NoError(err)

is.Run("default reuse-values", func() {
itest.TelepresenceOk(is.Context(), "helm", "upgrade", "--namespace", is.ManagerNamespace())
newValues, err := getValues()
if is.NoError(err) {
is.Equal(oldValues, newValues)
}
})

is.Run("default reset-values", func() {
// Setting a value means that the default behavior is to reset old values.
itest.TelepresenceOk(is.Context(), "helm", "upgrade", "--namespace", is.ManagerNamespace(), "--set", "apiPort=8765")
newValues, err := getValues()
if is.NoError(err) {
is.Equal(8765.0, newValues["apiPort"])
is.False(containsKey(newValues, "logLevel")) // Should be back at default
}
})

is.Run("explicit reuse-values", func() {
// Set new value and enforce merge with of old values.
itest.TelepresenceOk(is.Context(), "helm", "upgrade", "--namespace", is.ManagerNamespace(), "--set", "logLevel=debug", "--reuse-values")
newValues, err := getValues()
if is.NoError(err) {
is.Equal(8765.0, newValues["apiPort"])
is.Equal("debug", newValues["logLevel"])
}
})

is.Run("explicit reset-values", func() {
// Enforce reset of old values.
itest.TelepresenceOk(is.Context(), "helm", "upgrade", "--namespace", is.ManagerNamespace(), "--reset-values")
newValues, err := getValues()
if is.NoError(err) {
is.False(containsKey(newValues, "apiPort")) // Should be back at default
is.False(containsKey(newValues, "logLevel")) // Should be back at default
}
})
}

func (is *installSuite) Test_NonHelmInstall() {
ctx := is.Context()
require := is.Require()
Expand Down
48 changes: 24 additions & 24 deletions integration_test/limitrange_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,27 @@ import (
"github.com/telepresenceio/telepresence/v2/integration_test/itest"
)

func (s *installSuite) limitedRangeTest() {
func (is *installSuite) limitedRangeTest() {
const svc = "echo"
ctx := itest.WithUser(s.Context(), s.ManagerNamespace()+":"+itest.TestUser)
s.TelepresenceConnect(ctx)
ctx := itest.WithUser(is.Context(), is.ManagerNamespace()+":"+itest.TestUser)
is.TelepresenceConnect(ctx)
itest.TelepresenceOk(ctx, "loglevel", "debug")

require := s.Require()
itest.ApplyEchoService(ctx, svc, s.AppNamespace(), 8083)
require := is.Require()
itest.ApplyEchoService(ctx, svc, is.AppNamespace(), 8083)
defer func() {
s.NoError(itest.Kubectl(ctx, s.AppNamespace(), "delete", "svc,deploy", svc))
s.Eventually(func() bool { return len(itest.RunningPods(ctx, svc, s.AppNamespace())) == 0 }, 2*time.Minute, 6*time.Second)
is.NoError(itest.Kubectl(ctx, is.AppNamespace(), "delete", "svc,deploy", svc))
is.Eventually(func() bool { return len(itest.RunningPods(ctx, svc, is.AppNamespace())) == 0 }, 2*time.Minute, 6*time.Second)
}()

_, _, err := itest.Telepresence(ctx, "intercept", "--mount", "false", svc)
if err != nil {
if out, err := itest.KubectlOut(ctx, s.AppNamespace(), "get", "pod", "-o", "yaml", "-l", "app="+svc); err == nil {
if out, err := itest.KubectlOut(ctx, is.AppNamespace(), "get", "pod", "-o", "yaml", "-l", "app="+svc); err == nil {
dlog.Info(ctx, out)
}
}
require.NoError(err)
s.Eventually(
is.Eventually(
func() bool {
stdout, _, err := itest.Telepresence(ctx, "list", "--intercepts")
return err == nil && strings.Contains(stdout, svc+": intercepted")
Expand All @@ -44,7 +44,7 @@ func (s *installSuite) limitedRangeTest() {
itest.TelepresenceOk(ctx, "leave", svc)

// Ensure that LimitRange is injected into traffic-agent
out, err := itest.KubectlOut(ctx, s.AppNamespace(), "get", "pods", "-l", "app="+svc, "-o",
out, err := itest.KubectlOut(ctx, is.AppNamespace(), "get", "pods", "-l", "app="+svc, "-o",
`jsonpath={range .items.*.spec.containers[?(@.name=='traffic-agent')]}{.resources}{","}{end}`)
require.NoError(err)
dlog.Infof(ctx, "resources = %s", out)
Expand All @@ -60,28 +60,28 @@ func (s *installSuite) limitedRangeTest() {
require.True(m != nil && m.Equal(oneGig))
}

func (s *installSuite) TestLimitRange() {
ctx := s.Context()
require := s.Require()
require.NoError(itest.Kubectl(ctx, s.ManagerNamespace(), "apply", "-f", filepath.Join("testdata", "k8s", "client_sa.yaml")))
func (is *installSuite) TestLimitRange() {
ctx := is.Context()
require := is.Require()
require.NoError(itest.Kubectl(ctx, is.ManagerNamespace(), "apply", "-f", filepath.Join("testdata", "k8s", "client_sa.yaml")))
defer func() {
require.NoError(itest.Kubectl(ctx, s.ManagerNamespace(), "delete", "-f", filepath.Join("testdata", "k8s", "client_sa.yaml")))
require.NoError(itest.Kubectl(ctx, is.ManagerNamespace(), "delete", "-f", filepath.Join("testdata", "k8s", "client_sa.yaml")))
}()

defer s.UninstallTrafficManager(ctx, s.ManagerNamespace())
defer is.UninstallTrafficManager(ctx, is.ManagerNamespace())

require.NoError(itest.Kubectl(ctx, s.AppNamespace(), "apply", "-f", filepath.Join("testdata", "k8s", "memory-constraints.yaml")))
require.NoError(itest.Kubectl(ctx, is.AppNamespace(), "apply", "-f", filepath.Join("testdata", "k8s", "memory-constraints.yaml")))
defer func() {
require.NoError(itest.Kubectl(ctx, s.AppNamespace(), "delete", "-f", filepath.Join("testdata", "k8s", "memory-constraints.yaml")))
require.NoError(itest.Kubectl(ctx, is.AppNamespace(), "delete", "-f", filepath.Join("testdata", "k8s", "memory-constraints.yaml")))
}()

s.Run("Never", func() {
s.NoError(s.TelepresenceHelmInstall(s.Context(), false, "--set", "agentInjector.webhook.reinvocationPolicy=Never"))
s.limitedRangeTest()
is.Run("Never", func() {
is.NoError(is.TelepresenceHelmInstall(is.Context(), false, "--set", "agentInjector.webhook.reinvocationPolicy=Never"))
is.limitedRangeTest()
})

s.Run("IfNeeded", func() {
s.NoError(s.TelepresenceHelmInstall(s.Context(), true, "--set", "agentInjector.webhook.reinvocationPolicy=IfNeeded"))
s.limitedRangeTest()
is.Run("IfNeeded", func() {
is.NoError(is.TelepresenceHelmInstall(is.Context(), true, "--set", "agentInjector.webhook.reinvocationPolicy=IfNeeded"))
is.limitedRangeTest()
})
}
16 changes: 13 additions & 3 deletions pkg/client/cli/helm/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,25 @@ func ensureIsInstalled(
}

// OK, now install things.
var vals map[string]any
var providedVals map[string]any
if len(req.ValuesJson) > 0 {
if err := json.Unmarshal(req.ValuesJson, &vals); err != nil {
if err := json.Unmarshal(req.ValuesJson, &providedVals); err != nil {
return fmt.Errorf("unable to parse values JSON: %w", err)
}
vals = chartutil.CoalesceTables(vals, GetValuesFunc(ctx))
}

var vals map[string]any
if len(providedVals) > 0 {
vals = chartutil.CoalesceTables(providedVals, GetValuesFunc(ctx))
} else {
// No values were provided. This means that an upgrade should retain existing values unless
// reset-values is true.
if req.Type == Upgrade && !req.ResetValues {
req.ReuseValues = true
}
vals = GetValuesFunc(ctx)
}
dlog.Debugf(ctx, "reuse-values: %t, reset-values: %t, provided-values %s, values: %v", req.ReuseValues, req.ResetValues, providedVals, vals)

switch {
case existing == nil: // fresh install
Expand Down

0 comments on commit 030848a

Please sign in to comment.