Skip to content

check if the containers' image is downstream #23185

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 2 commits into from
Jul 2, 2019
Merged

check if the containers' image is downstream #23185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2f9c60
check if the containers'image is downstream
jianzhangbjz Jun 16, 2019
e32900f
modify the Run function
jianzhangbjz Jun 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions test/extended/operators/images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ import (

var _ = Describe("[Feature:Platform][Smoke] Managed cluster", func() {
oc := exutil.NewCLIWithoutNamespace("operators")

It("should ensure pods use images from our release image with proper ImagePullPolicy", func() {
It("should ensure pods use downstream images from our release image with proper ImagePullPolicy", func() {
imagePullSecret, err := oc.KubeFramework().ClientSet.CoreV1().Secrets("openshift-config").Get("pull-secret", metav1.GetOptions{})
if err != nil {
e2e.Failf("unable to get pull secret for cluster: %v", err)
Expand Down Expand Up @@ -78,6 +77,7 @@ var _ = Describe("[Feature:Platform][Smoke] Managed cluster", func() {
// list of pods that use images not in the release payload
invalidPodContainerImages := sets.NewString()
invalidPodContainerImagePullPolicy := sets.NewString()
invalidPodContainerDownstreamImages := sets.NewString()
// a pod in a namespace that begins with kube-* or openshift-* must come from our release payload
// TODO components in openshift-operators may not come from our payload, may want to weaken restriction
namespacePrefixes := sets.NewString("kube-", "openshift-")
Expand All @@ -99,20 +99,48 @@ var _ = Describe("[Feature:Platform][Smoke] Managed cluster", func() {
if !validImages.Has(container.Image) {
invalidPodContainerImages.Insert(fmt.Sprintf("%s/%s/%s image=%s", pod.Namespace, pod.Name, container.Name, container.Image))
}

if container.ImagePullPolicy != v1.PullIfNotPresent {
invalidPodContainerImagePullPolicy.Insert(fmt.Sprintf("%s/%s/%s imagePullPolicy=%s", pod.Namespace, pod.Name, container.Name, container.ImagePullPolicy))
}
}
// check if the container's image from the downstream.
for j := range pod.Spec.Containers {
containerName := pod.Spec.Containers[j].Name
commands := []string{
"exec",
pod.Name,
"-c",
containerName,
"--",
"cat",
"/etc/redhat-release",
}
oc.SetNamespace(pod.Namespace)
result, err := oc.AsAdmin().Run(commands...).Args().Output()
if err != nil {
e2e.Logf("unable to run command:%v with error: %v", commands, err)
continue
}
e2e.Logf("Image relase info:%s", result)
if !strings.Contains(result, "Red Hat Enterprise Linux Server") {
Copy link
Member

@liangxia liangxia Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not supporting RHEL CoreOS based images ?

Copy link
Member Author

@jianzhangbjz jianzhangbjz Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for now, I guess the release images have not based on the RHCOS yet. Correct me if I'm wrong. What do you think?@vikaslaad

Copy link
Contributor

@bparees bparees Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the images this would be checking are all UBI based and I'd expect them to continue to be.

Copy link
Contributor

@adambkaplan adambkaplan Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inferring from @smarterclayton - will this continue to be the case if we run OKD CI and with Fedora CoreOS as the base?

Copy link
Contributor

@bparees bparees Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fedora coreos would be the base for the nodes, not the container images which i would expect to continue to be UBI based. I'm not aware of any plans for a fedora equivalent of the UBI.

Copy link
Member Author

@jianzhangbjz jianzhangbjz Jun 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bparees I'm sorry, what're the checkpoints of UBI based images? Do we have any special label when running UBI images?

Copy link
Contributor

@bparees bparees Jun 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only run ubi images, there are no other images.

Copy link
Member Author

@jianzhangbjz jianzhangbjz Jun 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bparees I guess checking the output of $ cat /etc/redhat-release is enough for identifying the UBI images, what do you think?

Copy link
Contributor

@bparees bparees Jun 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jianzhangbjz i think it's fine for now, if it becomes a problem we can always revisit it.

Copy link
Member Author

@jianzhangbjz jianzhangbjz Jun 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks!

invalidPodContainerDownstreamImages.Insert(fmt.Sprintf("%s/%s invalid downstream image!", pod.Name, containerName))
}
}
}
}
// log for debugging output before we ultimately fail
e2e.Logf("Pods found with invalid container images not present in release payload: %s", strings.Join(invalidPodContainerImages.List(), "\n"))
e2e.Logf("Pods found with invalid container image pull policy not equal to IfNotPresent: %s", strings.Join(invalidPodContainerImagePullPolicy.List(), "\n"))
e2e.Logf("Pods with invalid dowstream images: %s", strings.Join(invalidPodContainerDownstreamImages.List(), "\n"))
if len(invalidPodContainerImages) > 0 {
e2e.Failf("Pods found with invalid container images not present in release payload: %s", strings.Join(invalidPodContainerImages.List(), "\n"))
}
if len(invalidPodContainerImagePullPolicy) > 0 {
e2e.Failf("Pods found with invalid container image pull policy not equal to IfNotPresent: %s", strings.Join(invalidPodContainerImagePullPolicy.List(), "\n"))
}
if len(invalidPodContainerDownstreamImages) > 0 {
e2e.Failf("Pods with invalid dowstream images: %s", strings.Join(invalidPodContainerDownstreamImages.List(), "\n"))
}
})
})
6 changes: 3 additions & 3 deletions test/extended/util/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,12 +473,12 @@ func (c *CLI) Run(commands ...string) *CLI {
adminConfigPath: c.adminConfigPath,
configPath: c.configPath,
username: c.username,
globalArgs: append(commands, []string{
globalArgs: append([]string{
fmt.Sprintf("--config=%s", c.configPath),
}...),
}, commands...),
}
if !c.withoutNamespace {
nc.globalArgs = append(nc.globalArgs, fmt.Sprintf("--namespace=%s", c.Namespace()))
nc.globalArgs = append([]string{fmt.Sprintf("--namespace=%s", c.Namespace())}, nc.globalArgs...)
}
nc.stdin, nc.stdout, nc.stderr = in, out, errout
return nc.setOutput(c.stdout)
Expand Down