Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,15 @@
"source": "openshift:payload:cluster-version-operator",
"lifecycle": "blocking",
"environmentSelector": {}
},
{
"name": "[Jira:\"Cluster Version Operator\"] cluster-version-operator-tests can use oc to get the version information",
"labels": {},
"resources": {
"isolation": {}
},
"source": "openshift:payload:cluster-version-operator",
"lifecycle": "blocking",
"environmentSelector": {}
}
]
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.24.0
require (
github.com/blang/semver/v4 v4.0.0
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/go-logr/logr v1.4.2
github.com/google/go-cmp v0.7.0
github.com/google/uuid v1.6.0
github.com/onsi/ginkgo/v2 v2.21.0
Expand Down Expand Up @@ -40,7 +41,6 @@ require (
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/emicklei/go-restful/v3 v3.12.2 // indirect
github.com/fxamacker/cbor/v2 v2.9.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
github.com/go-openapi/jsonreference v0.20.2 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
Expand Down
15 changes: 15 additions & 0 deletions test/cvo/cvo.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,25 @@ package cvo
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/openshift/cluster-version-operator/test/oc"
ocapi "github.com/openshift/cluster-version-operator/test/oc/api"
)

var logger = GinkgoLogr.WithName("cluster-version-operator-tests")

var _ = Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator-tests`, func() {
It("should support passing tests", func() {
Expect(true).To(BeTrue())
})

It("can use oc to get the version information", func() {
ocClient, err := oc.NewOC(logger)
Expect(err).NotTo(HaveOccurred())
Expect(ocClient).NotTo(BeNil())

output, err := ocClient.Version(ocapi.VersionOptions{Client: true})
Expect(err).NotTo(HaveOccurred())
Expect(output).To(ContainSubstring("Client Version:"))
})
})
14 changes: 14 additions & 0 deletions test/oc/api/api.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package api

type ReleaseExtractOptions struct {
To string
}

type VersionOptions struct {
Client bool
}

type OC interface {
AdmReleaseExtract(o ReleaseExtractOptions) error
Version(o VersionOptions) (string, error)
}
106 changes: 106 additions & 0 deletions test/oc/cli/cli.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package cli

import (
"context"
"errors"
"fmt"
"os"
"os/exec"
"strings"
"time"

"github.com/go-logr/logr"

"github.com/openshift/cluster-version-operator/test/oc/api"
)

type client struct {
logger logr.Logger
executor executor
}

type executor interface {
Run(args ...string) ([]byte, error)
}

type ocExecutor struct {
// logger is used to log oc commands
logger logr.Logger
// oc is the path to the oc binary
oc string
// execute executes a command
execute func(dir, command string, args ...string) ([]byte, error)
}

func (e *ocExecutor) Run(args ...string) ([]byte, error) {
Copy link
Contributor

@JianLi-RH JianLi-RH Dec 2, 2025

Choose a reason for hiding this comment

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

Why not implement Run on client directly?
In this PR, you introduced ocExecutor struct and executor interface, it looks overly complicated.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is my way/habit to layer things up.
When we have another ocExecutor implementation, we could hook it up easily.

Let us see if everyone is OK with this way, building up oc's features.
I can make it flat later (not a big deal to me) when the idea is accepted.
From my experience, we do not need to touch this part of code any more once merged.
All we need to do is to add new functions to the interface if needed and call the oc with the right sequence of args to implement it.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my experience, interfaces should only be introduced when there are many different instances, but for the CVO testing, there is only one OC client (we will not use client-go), so the interfaces are not necessary.

Leave it for @DavidHurta @jhou1 @jiajliu @shahsahil264 to confirm as you will use it in the future.

logger := e.logger.WithValues("cmd", e.oc, "args", strings.Join(args, " "))
b, err := e.execute("", e.oc, args...)
if err != nil {
logger.Error(err, "Running command failed", "output", string(b))
} else {
logger.Info("Running command succeeded.")
}
return b, err
}

func newOCExecutor(oc string, timeout time.Duration, logger logr.Logger) (executor, error) {
return &ocExecutor{
logger: logger,
oc: oc,
execute: func(dir, command string, args ...string) ([]byte, error) {
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
c := exec.CommandContext(ctx, command, args...)
c.Dir = dir
o, err := c.CombinedOutput()
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return o, fmt.Errorf("execution timed out after %s: %w", timeout.String(), ctx.Err())
}
return o, err
},
}, nil
}

// NewOCCli return a client for oc-cli.
func NewOCCli(logger logr.Logger) (api.OC, error) {
oc, err := exec.LookPath("oc")
if err != nil {
return nil, err
}
timeout := 30 * time.Second
timeoutStr := os.Getenv("OC_CLI_TIMEOUT")
if timeoutStr != "" {
timeout, err = time.ParseDuration(timeoutStr)
if err != nil {
return nil, err
}
}

executor, err := newOCExecutor(oc, timeout, logger)
if err != nil {
return nil, err
}
ret := client{
logger: logger,
executor: executor,
}
return &ret, nil
}

func (c *client) AdmReleaseExtract(o api.ReleaseExtractOptions) error {
args := []string{"adm", "release", "extract", fmt.Sprintf("--to=%s", o.To)}
_, err := c.executor.Run(args...)
if err != nil {
return err
}
return nil
}

func (c *client) Version(o api.VersionOptions) (string, error) {
args := []string{"version", fmt.Sprintf("--client=%t", o.Client)}
output, err := c.executor.Run(args...)
if err != nil {
return "", err
}
return string(output), nil
}
13 changes: 13 additions & 0 deletions test/oc/oc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package oc

import (
"github.com/go-logr/logr"

"github.com/openshift/cluster-version-operator/test/oc/api"
"github.com/openshift/cluster-version-operator/test/oc/cli"
)

// NewOC returns OC that provides utility functions used by the e2e tests
func NewOC(logger logr.Logger) (api.OC, error) {
return cli.NewOCCli(logger)
}