-
Notifications
You must be signed in to change notification settings - Fork 33
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 integration tests for grafana deployment #1156
Add integration tests for grafana deployment #1156
Conversation
Ran the test suite a few times, works as expected:
|
ed0168d
to
baac997
Compare
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.
Some changes are needed here, let me know if you want to sync up
6fbadde
to
3fc01a0
Compare
Tested again after addressing comments:
|
dff6bce
to
fab127e
Compare
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.
Overall looks much better, please address comments and build and image so I can run it in one of the basic jobs
pkg/util/test/util.go
Outdated
func ValidateGrafanaDeploymentImage(image string) error { | ||
check := func() (interface{}, bool, error) { | ||
pods, err := coreops.Instance().GetPods("kube-system", map[string]string{"app": "grafana"}) | ||
if err != nil { | ||
return "", true, err | ||
} | ||
|
||
podCount := len(pods.Items) | ||
if podCount > 0 && len(pods.Items[0].Spec.Containers) > 0 { | ||
actualImage := pods.Items[0].Spec.Containers[0].Image | ||
if actualImage == image { | ||
logrus.Infof("Grafana installed successfully with image %s", image) | ||
return "", false, nil | ||
} else { | ||
logrus.Errorf("Grafana not yet ready with image %s", image) | ||
return "", true, fmt.Errorf("grafana is not installed with expected image %s. actual: %s", image, actualImage) | ||
} | ||
} else { | ||
return "", true, fmt.Errorf("grafana is not installed with image %s. number of pods: %v", image, podCount) | ||
} | ||
} | ||
|
||
if _, err := task.DoRetryWithTimeout(check, 1*time.Minute, 10*time.Second); err != nil { | ||
return err | ||
} | ||
|
||
return nil | ||
} |
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.
Remove this since you using validateContainerImageInsidePods()
and we don't need this any more
pkg/util/test/util.go
Outdated
|
||
func ValidateGrafanaDeployment(cluster *corev1.StorageCluster, shouldBeInstalled bool, pxImageList map[string]string) error { | ||
check := func() (interface{}, bool, error) { | ||
pods, err := coreops.Instance().GetPods("kube-system", map[string]string{"app": "grafana"}) |
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.
Don't hardcode kube-system
, this can be installed into any namespace, let's get namespace from cluster.Namespace
and use that instead
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.
done
pkg/util/test/util.go
Outdated
if len(pods.Items) == 0 || pods.Items[0].Status.Phase != v1.PodRunning { | ||
logrus.Errorf("Grafana not yet ready") | ||
return "", true, fmt.Errorf("grafana is not installed when it should be") | ||
} |
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.
Is grafana a deployment? Maybe just using sched-ops
function ValidateDeployment
should be enough here as it would do all the pod validation, if its not and its just a pod, this should be ok
pkg/util/test/util.go
Outdated
logrus.Errorf("Grafana image is invalid yet ready") | ||
return "", true, fmt.Errorf("grafana is not installed when it should be") |
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.
Let's not log error and then return it, let's just return verbose error instead
pkg/util/test/util.go
Outdated
logrus.Errorf("Grafana not yet uninstalled") | ||
return "", true, fmt.Errorf("grafana is installed when it is expected to be uninstalled") |
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.
Same here and all other place you have this
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.
LGTM, let's run op-basic
job once with this private image before merging. Let me know when you have an image ready
Thanks, sent the test image. |
4039639
to
032506a
Compare
e85c1e9
to
c6efb64
Compare
cf214db
to
98eda84
Compare
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
Signed-off-by: Grant Griffiths <ggriffiths@purestorage.com>
98eda84
to
7e6da52
Compare
What this PR does / why we need it:
Adds a few tests to the operator integration test suite for validating grafana deployments, svcs, configmaps, and airgapped support.
Which issue(s) this PR fixes (optional)
Closes #
Special notes for your reviewer: