-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Add option for operator to integration tests #18991
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,8 +96,6 @@ mixer: | |
adapters: | ||
stdio: | ||
enabled: true | ||
kiali: | ||
enabled: true | ||
|
||
sidecarInjectorWebhook: | ||
rewriteAppHTTPProbe: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,6 +67,7 @@ var ( | |
PolicyNamespace: DefaultSystemNamespace, | ||
IngressNamespace: DefaultSystemNamespace, | ||
EgressNamespace: DefaultSystemNamespace, | ||
Operator: false, | ||
DeployIstio: true, | ||
DeployTimeout: 0, | ||
UndeployTimeout: 0, | ||
|
@@ -115,12 +116,20 @@ type Config struct { | |
// The Helm values file to be used. | ||
ValuesFile string | ||
|
||
// Override values specifically for the ICP crd | ||
// This is mostly required for cases where --set cannot be used | ||
// If specified, Values will be ignored | ||
ControlPlaneValues string | ||
|
||
// Overrides for the Helm values file. | ||
Values map[string]string | ||
|
||
// Indicates that the test should deploy Istio into the target Kubernetes cluster before running tests. | ||
DeployIstio bool | ||
|
||
// Operator determines if we should use the operator for installation | ||
Operator bool | ||
|
||
// Do not wait for the validation webhook before completing the deployment. This is useful for | ||
// doing deployments without Galley. | ||
SkipWaitForValidationWebhook bool | ||
|
@@ -161,6 +170,47 @@ func (c *Config) IsMtlsEnabled() bool { | |
return true | ||
} | ||
|
||
func (c *Config) IstioControlPlane() string { | ||
data := c.ControlPlaneValues | ||
if c.ValuesFile != "" { | ||
var err error | ||
data, err = file.AsString(filepath.Join(c.ChartDir, c.ValuesFile)) | ||
if err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log the error? |
||
return "" | ||
} | ||
} | ||
s, err := image.SettingsFromCommandLine() | ||
if err != nil { | ||
return "" | ||
} | ||
|
||
return fmt.Sprintf(` | ||
apiVersion: install.istio.io/v1alpha2 | ||
kind: IstioControlPlane | ||
spec: | ||
hub: %s | ||
tag: %s | ||
values: | ||
%s | ||
`, s.Hub, s.Tag, Indent(data, " ")) | ||
} | ||
|
||
// indents a block of text with an indent string | ||
func Indent(text, indent string) string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this need to be public? If so, seems pretty generic ... maybe move somewhere more common? |
||
if text[len(text)-1:] == "\n" { | ||
result := "" | ||
for _, j := range strings.Split(text[:len(text)-1], "\n") { | ||
result += indent + j + "\n" | ||
} | ||
return result | ||
} | ||
result := "" | ||
for _, j := range strings.Split(strings.TrimRight(text, "\n"), "\n") { | ||
result += indent + j + "\n" | ||
} | ||
return result[:len(result)-1] | ||
} | ||
|
||
// DefaultConfig creates a new Config from defaults, environments variables, and command-line parameters. | ||
func DefaultConfig(ctx resource.Context) (Config, error) { | ||
// Make a local copy. | ||
|
@@ -287,6 +337,7 @@ func (c *Config) String() string { | |
result += fmt.Sprintf("IngressNamespace: %s\n", c.IngressNamespace) | ||
result += fmt.Sprintf("EgressNamespace: %s\n", c.EgressNamespace) | ||
result += fmt.Sprintf("DeployIstio: %v\n", c.DeployIstio) | ||
result += fmt.Sprintf("Operator: %v\n", c.Operator) | ||
result += fmt.Sprintf("DeployTimeout: %s\n", c.DeployTimeout.String()) | ||
result += fmt.Sprintf("UndeployTimeout: %s\n", c.UndeployTimeout.String()) | ||
result += fmt.Sprintf("Values: %v\n", c.Values) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,7 +82,11 @@ func Deploy(ctx resource.Context, cfg *Config) (Instance, error) { | |
var i Instance | ||
switch ctx.Environment().EnvironmentName() { | ||
case environment.Kube: | ||
i, err = deploy(ctx, ctx.Environment().(*kube.Environment), *cfg) | ||
if cfg.Operator { | ||
i, err = deployOperator(ctx, ctx.Environment().(*kube.Environment), *cfg) | ||
} else { | ||
i, err = deploy(ctx, ctx.Environment().(*kube.Environment), *cfg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could rename |
||
} | ||
default: | ||
err = resource.UnsupportedEnvironment(ctx.Environment()) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
// Copyright 2019 Istio 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 istio | ||
|
||
import ( | ||
"fmt" | ||
"io" | ||
"io/ioutil" | ||
"os" | ||
"path" | ||
"path/filepath" | ||
|
||
"istio.io/istio/pkg/test/deployment" | ||
"istio.io/istio/pkg/test/framework/components/environment/kube" | ||
"istio.io/istio/pkg/test/framework/components/istioctl" | ||
"istio.io/istio/pkg/test/framework/core/image" | ||
"istio.io/istio/pkg/test/framework/resource" | ||
"istio.io/istio/pkg/test/scopes" | ||
) | ||
|
||
type operatorComponent struct { | ||
id resource.ID | ||
settings Config | ||
ctx resource.Context | ||
environment *kube.Environment | ||
} | ||
|
||
var _ io.Closer = &operatorComponent{} | ||
var _ Instance = &operatorComponent{} | ||
var _ resource.Dumper = &operatorComponent{} | ||
|
||
// ID implements resource.Instance | ||
func (i *operatorComponent) ID() resource.ID { | ||
return i.id | ||
} | ||
|
||
func (i *operatorComponent) Settings() Config { | ||
return i.settings | ||
} | ||
|
||
func (i *operatorComponent) Close() (err error) { | ||
scopes.CI.Infof("=== BEGIN: Cleanup Istio ===") | ||
defer scopes.CI.Infof("=== DONE: Cleanup Istio ===") | ||
if i.settings.DeployIstio { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should delete using the operator as well. An There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it work? That was the way helm was documented but the helm tests do a delete of the namespace, not sure if that was intentional. I guess testing the delete should be part of the test though, so if it doesn't work we need to fix it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep. deletion must be tested as well. |
||
err = i.environment.Accessor.DeleteNamespace(i.settings.SystemNamespace) | ||
if err == nil { | ||
err = i.environment.Accessor.WaitForNamespaceDeletion(i.settings.SystemNamespace) | ||
} | ||
// Note: when cleaning up an Istio deployment, ValidatingWebhookConfiguration | ||
// and MutatingWebhookConfiguration must be cleaned up. Otherwise, next | ||
// Istio deployment in the cluster will be impacted, causing flaky test results. | ||
// Clean up ValidatingWebhookConfiguration, if any | ||
_ = i.environment.DeleteValidatingWebhook(DefaultValidatingWebhookConfigurationName) | ||
// Clean up MutatingWebhookConfiguration, if any | ||
_ = i.environment.DeleteMutatingWebhook(DefaultMutatingWebhookConfigurationName) | ||
} | ||
return | ||
} | ||
|
||
func (i *operatorComponent) Dump() { | ||
scopes.CI.Errorf("=== Dumping Istio Deployment State...") | ||
|
||
d, err := i.ctx.CreateTmpDirectory("istio-state") | ||
if err != nil { | ||
scopes.CI.Errorf("Unable to create directory for dumping Istio contents: %v", err) | ||
return | ||
} | ||
|
||
deployment.DumpPodState(d, i.settings.SystemNamespace, i.environment.Accessor) | ||
deployment.DumpPodEvents(d, i.settings.SystemNamespace, i.environment.Accessor) | ||
|
||
pods, err := i.environment.Accessor.GetPods(i.settings.SystemNamespace) | ||
if err != nil { | ||
scopes.CI.Errorf("Unable to get pods from the system namespace: %v", err) | ||
return | ||
} | ||
|
||
for _, pod := range pods { | ||
for _, container := range pod.Spec.Containers { | ||
l, err := i.environment.Logs(pod.Namespace, pod.Name, container.Name, false /* previousLog */) | ||
if err != nil { | ||
scopes.CI.Errorf("Unable to get logs for pod/container: %s/%s/%s", pod.Namespace, pod.Name, container.Name) | ||
continue | ||
} | ||
|
||
fname := path.Join(d, fmt.Sprintf("%s-%s.log", pod.Name, container.Name)) | ||
if err = ioutil.WriteFile(fname, []byte(l), os.ModePerm); err != nil { | ||
scopes.CI.Errorf("Unable to write logs for pod/container: %s/%s/%s", pod.Namespace, pod.Name, container.Name) | ||
} | ||
} | ||
} | ||
} | ||
|
||
func deployOperator(ctx resource.Context, env *kube.Environment, cfg Config) (Instance, error) { | ||
scopes.CI.Infof("=== Istio Component Config ===") | ||
scopes.CI.Infof("\n%s", cfg.String()) | ||
scopes.CI.Infof("================================") | ||
|
||
i := &operatorComponent{ | ||
environment: env, | ||
settings: cfg, | ||
ctx: ctx, | ||
} | ||
i.id = ctx.TrackResource(i) | ||
|
||
if !cfg.DeployIstio { | ||
scopes.Framework.Info("skipping deployment due to Config") | ||
return i, nil | ||
} | ||
|
||
istioCtl, err := istioctl.New(ctx, istioctl.Config{}) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Top-level work dir for Istio deployment. | ||
workDir, err := ctx.CreateTmpDirectory("istio-deployment") | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
icpFile := filepath.Join(workDir, "icp.yaml") | ||
if err := ioutil.WriteFile(icpFile, []byte(cfg.IstioControlPlane()), os.ModePerm); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any concern about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I think this is actually normally returning |
||
return nil, fmt.Errorf("failed to write icp: %v", err) | ||
} | ||
s, err := image.SettingsFromCommandLine() | ||
if err != nil { | ||
return nil, err | ||
} | ||
cmd := []string{ | ||
"manifest", "apply", | ||
"--skip-confirmation", | ||
"--logtostderr", | ||
"-f", icpFile, | ||
"--force", // Blocked by https://github.com/istio/istio/issues/19009 | ||
"--set", "values.global.controlPlaneSecurityEnabled=false", | ||
"--set", "values.global.imagePullPolicy=" + s.PullPolicy, | ||
} | ||
// If control plane values set, assume this includes the full set of values, and .Values is | ||
// just for helm use case. Otherwise, include all values. | ||
if cfg.ControlPlaneValues == "" { | ||
for k, v := range cfg.Values { | ||
cmd = append(cmd, "--set", fmt.Sprintf("values.%s=%s", k, v)) | ||
} | ||
} | ||
|
||
scopes.CI.Infof("Running istioctl %v", cmd) | ||
if _, err := istioCtl.Invoke(cmd); err != nil { | ||
return nil, fmt.Errorf("manifest apply failed: %v", err) | ||
} | ||
|
||
if !cfg.SkipWaitForValidationWebhook { | ||
|
||
// Wait for Galley & the validation webhook to come online before continuing | ||
if _, _, err = env.WaitUntilServiceEndpointsAreReady(cfg.SystemNamespace, "istio-galley"); err != nil { | ||
err = fmt.Errorf("error waiting %s/istio-galley service endpoints: %v", cfg.SystemNamespace, err) | ||
scopes.CI.Info(err.Error()) | ||
i.Dump() | ||
return nil, err | ||
} | ||
|
||
// Wait for webhook to come online. The only reliable way to do that is to see if we can submit invalid config. | ||
err = waitForValidationWebhook(env.Accessor) | ||
if err != nil { | ||
i.Dump() | ||
return nil, err | ||
} | ||
} | ||
|
||
return i, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,6 @@ import ( | |
"istio.io/istio/pkg/test/framework/components/galley" | ||
"istio.io/istio/pkg/test/framework/components/ingress" | ||
"istio.io/istio/pkg/test/framework/components/istio" | ||
"istio.io/istio/pkg/test/framework/components/mixer" | ||
"istio.io/istio/pkg/test/framework/components/namespace" | ||
"istio.io/istio/pkg/test/framework/resource" | ||
util "istio.io/istio/tests/integration/mixer" | ||
|
@@ -94,9 +93,6 @@ func testsetup(ctx resource.Context) error { | |
return err | ||
} | ||
galInst = &g | ||
if _, err = mixer.New(ctx, mixer.Config{Galley: g}); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this factored in with the operator already ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not doing anything really except port forwarding to mixer then doing nothing. It's not actually installing mixer. I forget if I made this change to fix anything or if I was cleaning this up, I have been working on this off and on for a couple months |
||
return err | ||
} | ||
ing, err := ingress.New(ctx, ingress.Config{Istio: ist}) | ||
if err != nil { | ||
return err | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment that this will be ignored if
ValuesFile
is provided?