Skip to content

Commit b9c6614

Browse files
committed
Address comments from review
1 parent 7e63b07 commit b9c6614

3 files changed

Lines changed: 21 additions & 10 deletions

File tree

pkg/cli/admin/upgrade/accept/accept.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"github.com/google/go-cmp/cmp"
87
"strings"
98

9+
"github.com/google/go-cmp/cmp"
1010
configv1 "github.com/openshift/api/config/v1"
1111
configv1client "github.com/openshift/client-go/config/clientset/versioned"
1212
"github.com/spf13/cobra"
@@ -59,7 +59,7 @@ func New(f kcmdutil.Factory, streams genericiooptions.IOStreams) *cobra.Command
5959
Example: acceptExample,
6060
Run: func(cmd *cobra.Command, args []string) {
6161
kcmdutil.CheckErr(o.Complete(f, cmd, args))
62-
kcmdutil.CheckErr(o.Run(cmd.Context()))
62+
kcmdutil.CheckErr(o.Run())
6363
},
6464
}
6565

@@ -135,7 +135,8 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string
135135
return nil
136136
}
137137

138-
func (o *options) Run(ctx context.Context) error {
138+
func (o *options) Run() error {
139+
ctx := context.TODO()
139140
cv, err := o.Client.ConfigV1().ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
140141
if err != nil {
141142
if apierrors.IsNotFound(err) {
@@ -158,9 +159,9 @@ func (o *options) Run(ctx context.Context) error {
158159
for _, risk := range acceptRisks {
159160
names = append(names, risk.Name)
160161
}
161-
_, _ = fmt.Fprintf(o.Out, "info: Accept risks are [%s]\n", strings.Join(names, ", "))
162+
fmt.Fprintf(o.Out, "info: Accept risks are [%s]\n", strings.Join(names, ", ")) //nolint:errcheck
162163
} else {
163-
_, _ = fmt.Fprintf(o.Out, "info: Accept risks are not changed\n")
164+
fmt.Fprintf(o.Out, "info: Accept risks are not changed\n") //nolint:errcheck
164165
}
165166

166167
return nil

pkg/cli/admin/upgrade/rollback/rollback.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ func newOptions(streams genericiooptions.IOStreams) *options {
2727
func New(f kcmdutil.Factory, streams genericiooptions.IOStreams) *cobra.Command {
2828
o := newOptions(streams)
2929
cmd := &cobra.Command{
30-
Use: "rollback",
31-
Short: "Rollback the cluster to the previous release.",
30+
Use: "rollback",
31+
Hidden: true,
32+
Short: "Rollback the cluster to the previous release.",
3233
Long: templates.LongDesc(`
3334
Rollback the cluster to the previous release.
3435

test/e2e/accept.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55

66
g "github.com/onsi/ginkgo/v2"
7+
"github.com/onsi/ginkgo/v2/types"
78
o "github.com/onsi/gomega"
89

910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -14,7 +15,7 @@ import (
1415
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
1516
)
1617

17-
var _ = g.Describe("[sig-cli] oc", g.Label("cluster-version-operator"), func() {
18+
var _ = g.Describe("[sig-cli][OCPFeatureGate:ClusterUpdateAcceptRisks] oc", g.Label("cluster-version-operator"), func() {
1819

1920
var (
2021
ctx = context.TODO()
@@ -29,6 +30,9 @@ var _ = g.Describe("[sig-cli] oc", g.Label("cluster-version-operator"), func() {
2930
configClient, err = configv1client.NewForConfig(config)
3031
o.Expect(err).NotTo(o.HaveOccurred())
3132

33+
skipIfMicroShift(oc)
34+
SkipIfNotTechPreviewNoUpgrade(ctx, configClient)
35+
3236
cv, err := configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
3337
o.Expect(err).NotTo(o.HaveOccurred())
3438
if cv.Spec.DesiredUpdate != nil {
@@ -37,6 +41,13 @@ var _ = g.Describe("[sig-cli] oc", g.Label("cluster-version-operator"), func() {
3741
})
3842

3943
g.AfterEach(func() {
44+
// No need to recover if the test is skipped.
45+
// In case it is skipped up to disabled TP, we must not recover as the API might not be available.
46+
// We could use patch instead but this guard should work too.
47+
report := g.CurrentSpecReport()
48+
if report.State.Is(types.SpecStateSkipped) {
49+
return
50+
}
4051
cv, err := configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
4152
o.Expect(err).NotTo(o.HaveOccurred())
4253
if cv.Spec.DesiredUpdate != nil && len(cv.Spec.DesiredUpdate.AcceptRisks) > 0 {
@@ -48,8 +59,6 @@ var _ = g.Describe("[sig-cli] oc", g.Label("cluster-version-operator"), func() {
4859
})
4960

5061
g.It("can operate accept risks [Serial]", g.Label("tech-preview"), oteginkgo.Informing(), func() {
51-
skipIfMicroShift(oc)
52-
SkipIfNotTechPreviewNoUpgrade(ctx, configClient)
5362

5463
g.By("accepting some risks")
5564
out, err := oc.Run("adm").Args("upgrade", "accept", "RiskA,RiskB").WithoutNamespace().Output()

0 commit comments

Comments
 (0)