Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ require (
github.com/RangelReale/osincli v0.0.0-20160924135400-fababb0555f2
github.com/apparentlymart/go-cidr v1.1.0
github.com/aws/aws-sdk-go v1.50.25
github.com/blang/semver/v4 v4.0.0
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/distribution/distribution/v3 v3.0.0-20230530204932-ba46c769b3d1
github.com/fsouza/go-dockerclient v1.12.0
Expand Down Expand Up @@ -130,7 +131,6 @@ require (
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/campoy/embedmd v1.0.0 // indirect
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
Expand Down
293 changes: 293 additions & 0 deletions test/extended/cli/adm_upgrade/recommend.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
package adm_upgrade

import (
"context"
"fmt"
"net"
"net/url"
"strconv"
"strings"
"text/template"
"time"

"github.com/blang/semver/v4"
g "github.com/onsi/ginkgo/v2"
o "github.com/onsi/gomega"
configv1 "github.com/openshift/api/config/v1"
exutil "github.com/openshift/origin/test/extended/util"
"github.com/openshift/origin/test/extended/util/image"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/test/e2e/framework"
)

var _ = g.Describe("[Serial][sig-cli] oc adm upgrade recommend", g.Ordered, func() {
defer g.GinkgoRecover()
ctx := context.Background()

f := framework.NewDefaultFramework("oc-adm-upgrade-recommend")
oc := exutil.NewCLIWithFramework(f).AsAdmin()
var cv *configv1.ClusterVersion
var restoreChannel, restoreUpstream bool

g.BeforeAll(func() {
isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isMicroShift {
g.Skip("MicroShift does not have a ClusterVersion resource")
}

cv, err = oc.AdminConfigClient().ConfigV1().ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
o.Expect(err).NotTo(o.HaveOccurred())
})

g.AfterAll(func() {
if restoreChannel {
oc.Run("adm", "upgrade", "channel", cv.Spec.Channel).Execute()
}

if restoreUpstream {
oc.Run("patch", "clusterversions.config.openshift.io", "version", "--type", "json", "-p", fmt.Sprintf(`[{"op": "add", "path": "/spec/upstream", "value": "%s"}]`, cv.Spec.Upstream)).Execute()
}
})

g.It("runs successfully, even without upstream OpenShift Update Service customization", func() {
_, err := oc.Run("adm", "upgrade", "recommend").EnvVar("OC_ENABLE_CMD_UPGRADE_RECOMMEND", "true").Output()
o.Expect(err).NotTo(o.HaveOccurred())
})

g.It("runs successfully with an empty channel", func() {
err := oc.Run("adm", "upgrade", "channel").Execute()
if err != nil {
g.Skip(fmt.Sprintf("failed to update the ClusterVersion channel (perhaps we are on a HyperShift cluster): %s", err))
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to use the function exutil.IsHypershift to determine it (and then skip running the command of clearing the channel)?

Copy link
Member Author

Choose a reason for hiding this comment

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

There may be other systems that block our ability to set the channel via ClusterVersion (maybe Classic ROSA?), so I'd rather keep this generic for "do the thing we need to do to run the tests, and if it doesn't work, skip"). Unless we see that pattern breaking down somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

If we skip on errors, do we have a chance to spot the breaking down pattern?
Or that is just to say, we skip the case until someone reports we should not? <- This works for me too.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to start failing CI suites over issues that are cluster-side and orthogonal to the oc adm upgrade recommend we're trying to exercise. So skips to be non-fatal, and then we can periodically check to ensure we have sufficient non-skip passing coverage. If those checks turn up surprising numbers of skips, we can dig in and see if we want to make adjustments.

}
restoreChannel = true

out, err := oc.Run("adm", "upgrade", "recommend").EnvVar("OC_ENABLE_CMD_UPGRADE_RECOMMEND", "true").Output()
o.Expect(err).NotTo(o.HaveOccurred())
err = matchRegexp(out, `.*The update channel has not been configured.*`)
o.Expect(err).NotTo(o.HaveOccurred())
})

g.Context("When the update service has no recommendations", func() {
g.BeforeAll(func() {
isHyperShift, err := exutil.IsHypershift(ctx, oc.AdminConfigClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isHyperShift {
g.Skip("HyperShift does not support configuring the upstream OpenShift Update Service directoly via ClusterVersion (it must be configured via HostedCluster on the management cluster)")
}

graph := fmt.Sprintf(`{"nodes": [{"version": "%s","payload": "%s", "metadata": {"io.openshift.upgrades.graph.release.channels": "test-channel,other-channel"}}]}`, cv.Status.Desired.Version, cv.Status.Desired.Image)
newUpstream, err := runUpdateService(ctx, oc, graph)
o.Expect(err).NotTo(o.HaveOccurred())

err = oc.Run("adm", "upgrade", "channel", "test-channel").Execute()
if err != nil {
g.Skip(fmt.Sprintf("failed to update the ClusterVersion channel: %s", err))
}
restoreChannel = true

err = oc.Run("patch", "clusterversions.config.openshift.io", "version", "--type", "json", "-p", fmt.Sprintf(`[{"op": "add", "path": "/spec/upstream", "value": "%s"}]`, newUpstream.String())).Execute()
if err != nil {
g.Skip(fmt.Sprintf("failed to update the ClusterVersion upstream: %s", err))
}
restoreUpstream = true

time.Sleep(16 * time.Second) // Give the CVO time to retrieve recommendations and push to status
})

g.It("runs successfully", func() {
out, err := oc.Run("adm", "upgrade", "recommend").EnvVar("OC_ENABLE_CMD_UPGRADE_RECOMMEND", "true").Output()
o.Expect(err).NotTo(o.HaveOccurred())
err = matchRegexp(out, `.*Upstream update service: http://.*
Channel: test-channel [(]available channels: other-channel, test-channel[)]
No updates available. You may still upgrade to a specific release image.*`)
o.Expect(err).NotTo(o.HaveOccurred())
})
})

g.Context("When the update service has conditional recommendations", func() {
var currentVersion *semver.Version

g.BeforeAll(func() {
isHyperShift, err := exutil.IsHypershift(ctx, oc.AdminConfigClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isHyperShift {
g.Skip("HyperShift does not support configuring the upstream OpenShift Update Service directoly via ClusterVersion (it must be configured via HostedCluster on the management cluster)")
}

if curVer, err := semver.Parse(cv.Status.Desired.Version); err != nil {
o.Expect(err).NotTo(o.HaveOccurred())
} else {
currentVersion = &curVer
}

var buf strings.Builder
err = template.Must(template.New("letter").Parse(`{
"nodes": [
{"version": "{{.CurrentVersion}}","payload": "{{.CurrentImage}}", "metadata": {"io.openshift.upgrades.graph.release.channels": "test-channel,other-channel"}},
{"version": "4.{{.CurrentMinor}}.998","payload": "example.com/test@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
{"version": "4.{{.CurrentMinor}}.999","payload": "example.com/test@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"},
{"version": "4.{{.NextMinor}}.0","payload": "example.com/test@sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", "metadata": {"url": "https://example.com/release/4.{{.NextMinor}}.0"}}
],
"edges": [[0, 1], [0, 2]],
"conditionalEdges": [
{
"edges": [{"from": "{{.CurrentVersion}}", "to": "4.{{.NextMinor}}.0"}],
"risks": [
{
"url": "https://example.com/testRiskA",
"name": "TestRiskA",
"message": "This is a test risk.",
"matchingRules": [{"type": "PromQL", "promql": {"promql": "group(cluster_version)"}}]
}
]
}
]
}`)).Execute(&buf, struct {
CurrentVersion string
CurrentImage string
CurrentMinor uint64
NextMinor uint64
}{
CurrentVersion: cv.Status.Desired.Version,
CurrentImage: cv.Status.Desired.Image,
CurrentMinor: currentVersion.Minor,
NextMinor: currentVersion.Minor + 1,
})
o.Expect(err).NotTo(o.HaveOccurred())
graph := buf.String()

newUpstream, err := runUpdateService(ctx, oc, graph)
o.Expect(err).NotTo(o.HaveOccurred())

err = oc.Run("adm", "upgrade", "channel", "test-channel").Execute()
if err != nil {
g.Skip(fmt.Sprintf("failed to update the ClusterVersion channel: %s", err))
}
restoreChannel = true

err = oc.Run("patch", "clusterversions.config.openshift.io", "version", "--type", "json", "-p", fmt.Sprintf(`[{"op": "add", "path": "/spec/upstream", "value": "%s"}]`, newUpstream.String())).Execute()
if err != nil {
g.Skip(fmt.Sprintf("failed to update the ClusterVersion upstream: %s", err))
}
restoreUpstream = true

time.Sleep(16 * time.Second) // Give the CVO time to retrieve recommendations and push to status
})

g.It("runs successfully when listing all updates", func() {
out, err := oc.Run("adm", "upgrade", "recommend").EnvVar("OC_ENABLE_CMD_UPGRADE_RECOMMEND", "true").Output()
o.Expect(err).NotTo(o.HaveOccurred())
err = matchRegexp(out, `Upstream update service: http://.*
Channel: test-channel [(]available channels: other-channel, test-channel[)]

Updates to 4.[0-9]*:

Version: 4[.][0-9]*[.]0
Image: example[.]com/test@sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc
Reason: (TestRiskA|MultipleReasons)
Message: (?s:.*)This is a test risk[.] https://example[.]com/testRiskA

Updates to 4[.][0-9]*:
VERSION *ISSUES
4[.][0-9]*[.]999 *no known issues relevant to this cluster
4[.][0-9]*[.]998 *no known issues relevant to this cluster`)
o.Expect(err).NotTo(o.HaveOccurred())
})

g.It("runs successfully with conditional recommendations to the --version target", func() {
out, err := oc.Run("adm", "upgrade", "recommend", "--version", fmt.Sprintf("4.%d.0", currentVersion.Minor+1)).EnvVar("OC_ENABLE_CMD_UPGRADE_RECOMMEND", "true").Output()
o.Expect(err).NotTo(o.HaveOccurred())
err = matchRegexp(out, `Upstream update service: http://.*
Channel: test-channel [(]available channels: other-channel, test-channel[)]

Update to 4[.][0-9]*[.]0 Recommended=False:
Image: example.com/test@sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc
Release URL: https://example.com/release/4[.][0-9]*[.]0
Reason: (TestRiskA|MultipleReasons)
Message: (?s:.*)This is a test risk. https://example.com/testRiskA`)
o.Expect(err).NotTo(o.HaveOccurred())
})
})
})

func runUpdateService(ctx context.Context, oc *exutil.CLI, graph string) (*url.URL, error) {
deployment, err := oc.AdminKubeClient().AppsV1().Deployments(oc.Namespace()).Create(ctx,
&appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-update-service-",
},
Spec: appsv1.DeploymentSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": "test-update-service",
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"app": "test-update-service",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: "update-service",
Image: image.ShellImage(),
Args: []string{
"/bin/sh",
"-c",
fmt.Sprintf(`DIR="$(mktemp -d)" &&
cd "${DIR}" &&
printf '%%s' '%s' >graph &&
python3 -m http.server --bind ::
`, graph),
},
Ports: []corev1.ContainerPort{{
Name: "update-service",
ContainerPort: 8000,
}},
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("10m"),
corev1.ResourceMemory: resource.MustParse("20Mi"),
},
},
}},
},
},
},
}, metav1.CreateOptions{})
if err != nil {
return nil, err
}

service, err := oc.AdminKubeClient().CoreV1().Services(oc.Namespace()).Create(ctx,
&corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: deployment.ObjectMeta.Name,
},
Spec: corev1.ServiceSpec{
Selector: deployment.Spec.Template.ObjectMeta.Labels,
Ports: []corev1.ServicePort{{
Name: deployment.Spec.Template.Spec.Containers[0].Ports[0].Name,
Port: deployment.Spec.Template.Spec.Containers[0].Ports[0].ContainerPort,
}},
},
}, metav1.CreateOptions{})
if err != nil {
return nil, err
}

if err = exutil.WaitForDeploymentReady(oc, deployment.ObjectMeta.Name, oc.Namespace(), -1); err != nil {
return nil, err
}

return &url.URL{
Scheme: "http",
Host: net.JoinHostPort(service.Spec.ClusterIP, strconv.Itoa(int(service.Spec.Ports[0].Port))),
Path: "graph",
}, nil
}
41 changes: 41 additions & 0 deletions test/extended/cli/adm_upgrade/regexp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package adm_upgrade

import (
"fmt"
"regexp"
"strings"
)

// matchRegexp returns nil if the input string matches the pattern.
// If the input string does not match the pattern, it returns an error
// describing the difference.
func matchRegexp(input, pattern string) error {
re, err := regexp.Compile(pattern)
if err != nil {
return err
}

if re.MatchString(input) {
return nil
}
patternFragment := pattern

// the full pattern did not match. Trim off portions until we find
// a match, to identify the point of divergence.
for {
if len(patternFragment) == 0 {
break
}
patternFragment = patternFragment[:len(patternFragment)-1]

re, err = regexp.Compile(patternFragment)
if err != nil {
continue
}

if re.MatchString(input) {
return fmt.Errorf("expected:\n %s\nto match regular expression:\n %s\nbut the longest pattern subset is:\n %s\nand the unmatched portion of the input is:\n %s", strings.ReplaceAll(input, "\n", "\n "), strings.ReplaceAll(pattern, "\n", "\n "), strings.ReplaceAll(patternFragment, "\n", "\n "), strings.ReplaceAll(re.ReplaceAllString(input, "<MATCHED>"), "\n", "\n "))
}
}
return fmt.Errorf("expected:\n %s\nto match regular expression:\n %s\nbut no pattern subsets found any matches.", strings.ReplaceAll(input, "\n", "\n "), strings.ReplaceAll(pattern, "\n", "\n "))
}
10 changes: 10 additions & 0 deletions test/extended/util/annotate/generated/zz_generated.annotations.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.