Skip to content

[run bundle] refactor catalog source and registry pod creation #3832

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
rashmigottipati merged 1 commit into from
Sep 8, 2020
Merged

[run bundle] refactor catalog source and registry pod creation #3832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
108 changes: 44 additions & 64 deletions internal/olm/operator/registry/index/registry_pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,20 @@ import (
"context"
"errors"
"fmt"
"path"
"strings"
"text/template"
"time"

"github.com/operator-framework/api/pkg/operators/v1alpha1"
log "github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/operator-framework/operator-sdk/internal/olm/operator"
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
)

Expand Down Expand Up @@ -71,21 +75,17 @@ type RegistryPod struct {
// if an index image is provided, the existing registry DB is located at /database/index.db
DBPath string

// Namespace refers to the specific namespace in which the registry pod will be created and scoped to
Namespace string

// GRPCPort is the container grpc port
GRPCPort int32

// client refers to a controller runtime client
client client.Client

// pod represents a kubernetes *corev1.pod that will be created on a cluster using an index image
pod *corev1.Pod

cfg *operator.Configuration
}

// NewRegistryPod initializes the RegistryPod struct and sets defaults for empty fields
func NewRegistryPod(client client.Client, dbPath, bundleImage, namespace string) (*RegistryPod, error) {
func NewRegistryPod(cfg *operator.Configuration, dbPath, bundleImage string) (*RegistryPod, error) {
rp := &RegistryPod{}

if rp.GRPCPort == 0 {
Expand All @@ -104,76 +104,63 @@ func NewRegistryPod(client client.Client, dbPath, bundleImage, namespace string)
}
}

rp.client = client
rp.cfg = cfg
rp.DBPath = dbPath
rp.BundleImage = bundleImage
rp.Namespace = namespace

// validate the RegistryPod struct and ensure required fields are set
if err := rp.validate(); err != nil {
return nil, fmt.Errorf("error in validating registry pod struct: %v", err)
return nil, fmt.Errorf("error validating registry pod struct: %v", err)
}

// call podForBundleRegistry() to make the pod definition
// podForBundleRegistry() to make the pod definition
pod, err := rp.podForBundleRegistry()
if err != nil {
return nil, fmt.Errorf("error in building registry pod definition: %v", err)
return nil, fmt.Errorf("error building registry pod definition: %v", err)
}
rp.pod = pod

return rp, nil
}

// Create creates a bundle registry pod built from an index image
// and returns error
func (rp *RegistryPod) Create(ctx context.Context) error {
// Create creates a bundle registry pod built from an index image,
// sets the catalog source as the owner for the pod and verifies that
// the pod is running
func (rp *RegistryPod) Create(ctx context.Context, cs *v1alpha1.CatalogSource) (*corev1.Pod, error) {
if rp.pod == nil {
return errPodNotInit
}

podKey, err := client.ObjectKeyFromObject(rp.pod)
if err != nil {
return fmt.Errorf("error in getting object key from the registry pod name %s: %v", rp.pod.Name, err)
return nil, errPodNotInit
}

if err := rp.client.Get(ctx, podKey, rp.pod); err != nil {
if k8serrors.IsNotFound(err) {
if err = rp.client.Create(ctx, rp.pod); err != nil {
return fmt.Errorf("error creating registry pod: %v", err)
}
} else {
return fmt.Errorf("registry pod name %s already exists: %v", rp.pod.Name, err)
}
// make catalog source the owner of registry pod object
if err := controllerutil.SetOwnerReference(cs, rp.pod, rp.cfg.Scheme); err != nil {
return nil, fmt.Errorf("set registry pod owner reference: %v", err)
}
return nil
}

// VerifyPodRunning calls checkPodStatus to verify pod status
// and returns error if pod is not running
func (rp *RegistryPod) VerifyPodRunning(ctx context.Context) error {
if rp.pod == nil {
return errPodNotInit
if err := rp.cfg.Client.Create(ctx, rp.pod); err != nil {
return nil, fmt.Errorf("create registry pod: %v", err)
}

podKey, err := client.ObjectKeyFromObject(rp.pod)
if err != nil {
return fmt.Errorf("error in getting object key from the registry pod name %s: %v", rp.pod.Name, err)
// get registry pod key
podKey := types.NamespacedName{
Namespace: rp.cfg.Namespace,
Name: getPodName(rp.BundleImage),
}

// upon creation of new pod, poll and verify that pod status is running
// poll and verify that pod is running
podCheck := wait.ConditionFunc(func() (done bool, err error) {
err = rp.client.Get(ctx, podKey, rp.pod)
err = rp.cfg.Client.Get(ctx, podKey, rp.pod)
if err != nil {
return false, fmt.Errorf("error getting pod %s: %w", rp.pod.Name, err)
}
return rp.pod.Status.Phase == corev1.PodRunning, nil
})

// check pod status to be Running
// check pod status to be `Running`
if err := rp.checkPodStatus(ctx, podCheck); err != nil {
return fmt.Errorf("registry pod did not become ready: %w", err)
return nil, fmt.Errorf("registry pod did not become ready: %w", err)
}
return nil
log.Infof("Successfully created registry pod: %s", rp.pod.Name)
return rp.pod, nil
}

// checkPodStatus polls and verifies that the pod status is running
Expand All @@ -197,10 +184,6 @@ func (rp *RegistryPod) validate() error {
return errors.New("registry database path cannot be empty")
}

if len(strings.TrimSpace(rp.Namespace)) < 1 {
return errors.New("pod namespace cannot be empty")
}

if len(strings.TrimSpace(rp.BundleAddMode)) < 1 {
return errors.New("bundle add mode cannot be empty")
}
Expand Down Expand Up @@ -230,14 +213,14 @@ func (rp *RegistryPod) podForBundleRegistry() (*corev1.Pod, error) {
// construct the container command for pod spec
containerCmd, err := rp.getContainerCmd()
if err != nil {
return nil, fmt.Errorf("error in parsing container command: %v", err)
return nil, fmt.Errorf("error parsing container command: %v", err)
}

// make the pod definition
rp.pod = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: getPodName(rp.BundleImage),
Namespace: rp.Namespace,
Namespace: rp.cfg.Namespace,
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
Expand All @@ -263,7 +246,7 @@ func (rp *RegistryPod) podForBundleRegistry() (*corev1.Pod, error) {
// getContainerCmd uses templating to construct the container command
// and throws error if unable to parse and execute the container command
func (rp *RegistryPod) getContainerCmd() (string, error) {
const containerCommand = "/bin/mkdir -p {{ .DBPath }} &&" +
const containerCommand = "/bin/mkdir -p {{ .DBPath | dirname }} &&" +
Copy link
Copy Markdown
Member

@estroz estroz Sep 8, 2020

Choose a reason for hiding this comment

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

NB: this may cause a similar issue in OCP described here.

"/bin/opm registry add -d {{ .DBPath }} -b {{.BundleImage}} --mode={{.BundleAddMode}} &&" +
"/bin/opm registry serve -d {{ .DBPath }} -p {{.GRPCPort}}"
type bundleCmd struct {
Expand All @@ -276,23 +259,20 @@ func (rp *RegistryPod) getContainerCmd() (string, error) {

out := &bytes.Buffer{}

// add the custom basename template function to the
// create a custom dirname template function
funcMap := template.FuncMap{
"dirname": path.Dir,
}

// add the custom dirname template function to the
// template's FuncMap and parse the containerCommand
tmp := template.Must(template.New("containerCommand").Parse(containerCommand))
tmp := template.Must(template.New("containerCommand").Funcs(funcMap).Parse(containerCommand))

// execute the command by applying the parsed tmp to command
// and write command output to out
if err := tmp.Execute(out, command); err != nil {
return "", fmt.Errorf("error in parsing container command: %w", err)
return "", fmt.Errorf("parse container command: %w", err)
}

return out.String(), nil
}

// GetPod returns the registry pod
func (rp *RegistryPod) GetPod() (*corev1.Pod, error) {
if rp == nil {
return nil, errPodNotInit
}
return rp.pod, nil
}
62 changes: 30 additions & 32 deletions internal/olm/operator/registry/index/registry_pod_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/operator-framework/operator-sdk/internal/olm/operator"
"k8s.io/apimachinery/pkg/util/wait"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -44,15 +46,20 @@ var _ = Describe("RegistryPod", func() {

Context("with valid registry pod values", func() {
expectedPodName := "quay-io-example-example-operator-bundle-0-2-0"
expectedOutput := "/bin/mkdir -p /database/index.db &&" +
expectedOutput := "/bin/mkdir -p /database &&" +
"/bin/opm registry add -d /database/index.db -b quay.io/example/example-operator-bundle:0.2.0 --mode=semver &&" +
"/bin/opm registry serve -d /database/index.db -p 50051"

var rp *RegistryPod
var cfg *operator.Configuration
var err error

BeforeEach(func() {
rp, err = NewRegistryPod(newFakeClient(), "/database/index.db", "quay.io/example/example-operator-bundle:0.2.0", "default")
cfg = &operator.Configuration{
Client: newFakeClient(),
Namespace: "test-default",
}
rp, err = NewRegistryPod(cfg, "/database/index.db", "quay.io/example/example-operator-bundle:0.2.0")
Expect(err).To(BeNil())
})

Expand All @@ -65,7 +72,7 @@ var _ = Describe("RegistryPod", func() {
It("should create the RegistryPod successfully", func() {
Expect(rp).NotTo(BeNil())
Expect(rp.pod.Name).To(Equal(expectedPodName))
Expect(rp.pod.Namespace).To(Equal(rp.Namespace))
Expect(rp.pod.Namespace).To(Equal(rp.cfg.Namespace))
Expect(rp.pod.Spec.Containers[0].Name).To(Equal(defaultContainerName))
if len(rp.pod.Spec.Containers) > 0 {
if len(rp.pod.Spec.Containers[0].Ports) > 0 {
Expand All @@ -86,7 +93,7 @@ var _ = Describe("RegistryPod", func() {

Expect(rp.pod).NotTo(BeNil())
Expect(rp.pod.Name).To(Equal(expectedPodName))
Expect(rp.pod.Namespace).To(Equal(rp.Namespace))
Expect(rp.pod.Namespace).To(Equal(rp.cfg.Namespace))
Expect(rp.pod.Spec.Containers[0].Name).To(Equal(defaultContainerName))
if len(rp.pod.Spec.Containers) > 0 {
if len(rp.pod.Spec.Containers[0].Ports) > 0 {
Expand All @@ -95,12 +102,6 @@ var _ = Describe("RegistryPod", func() {
}
})

It("should create registry pod successfully", func() {
err := rp.Create(context.Background())

Expect(err).To(BeNil())
})

It("check pod status should return successfully when pod check is true", func() {
mockGoodPodCheck := wait.ConditionFunc(func() (done bool, err error) {
return true, nil
Expand All @@ -113,22 +114,18 @@ var _ = Describe("RegistryPod", func() {
})

Context("with invalid registry pod values", func() {
var cfg *operator.Configuration
BeforeEach(func() {
cfg = &operator.Configuration{
Client: newFakeClient(),
Namespace: "test-default",
}
})

It("should error when bundle image is not provided", func() {
expectedErr := "bundle image cannot be empty"

_, err := NewRegistryPod(newFakeClient(), "/database/index.db",
"", "default")

Expect(err).NotTo(BeNil())
Expect(err.Error()).Should(ContainSubstring(expectedErr))
})

It("should not create a registry pod when namespace is not provided", func() {
expectedErr := "namespace cannot be empty"

_, err := NewRegistryPod(newFakeClient(), "/database/index.db",
"quay.io/example/example-operator-bundle:0.2.0", "")
_, err := NewRegistryPod(cfg, "/database/index.db", "")

Expect(err).NotTo(BeNil())
Expect(err.Error()).Should(ContainSubstring(expectedErr))
Expand All @@ -137,8 +134,8 @@ var _ = Describe("RegistryPod", func() {
It("should not create a registry pod when database path is not provided", func() {
expectedErr := "registry database path cannot be empty"

_, err := NewRegistryPod(newFakeClient(), "",
"quay.io/example/example-operator-bundle:0.2.0", "default")
_, err := NewRegistryPod(cfg, "",
"quay.io/example/example-operator-bundle:0.2.0")

Expect(err).NotTo(BeNil())
Expect(err.Error()).Should(ContainSubstring(expectedErr))
Expand All @@ -147,8 +144,8 @@ var _ = Describe("RegistryPod", func() {
It("should not create a registry pod when bundle add mode is empty", func() {
expectedErr := "bundle add mode cannot be empty"

rp, _ := NewRegistryPod(newFakeClient(), "/database/index.db",
"quay.io/example/example-operator-bundle:0.2.0", "default")
rp, _ := NewRegistryPod(cfg, "/database/index.db",
"quay.io/example/example-operator-bundle:0.2.0")
rp.BundleAddMode = ""

err := rp.validate()
Expand All @@ -159,8 +156,8 @@ var _ = Describe("RegistryPod", func() {
It("should not accept any other bundle add mode other than semver or replaces", func() {
expectedErr := "invalid bundle mode"

rp, _ := NewRegistryPod(newFakeClient(), "/database/index.db",
"quay.io/example/example-operator-bundle:0.2.0", "default")
rp, _ := NewRegistryPod(cfg, "/database/index.db",
"quay.io/example/example-operator-bundle:0.2.0")
rp.BundleAddMode = "invalid"

err := rp.validate()
Expand All @@ -169,8 +166,8 @@ var _ = Describe("RegistryPod", func() {
})

It("checkPodStatus should return error when pod check is false and context is done", func() {
rp, _ := NewRegistryPod(newFakeClient(), "/database/index.db",
"quay.io/example/example-operator-bundle:0.2.0", "default")
rp, _ := NewRegistryPod(cfg, "/database/index.db",
"quay.io/example/example-operator-bundle:0.2.0")

mockBadPodCheck := wait.ConditionFunc(func() (done bool, err error) {
return false, fmt.Errorf("error waiting for registry pod")
Expand All @@ -189,13 +186,14 @@ var _ = Describe("RegistryPod", func() {

It("Create should fail when registry pod is not initialized", func() {
rp := RegistryPod{}
err := rp.Create(context.Background())
cs := &v1alpha1.CatalogSource{}
pod, err := rp.Create(context.Background(), cs)

Expect(err).NotTo(BeNil())
Expect(pod).To(BeNil())
Expect(err).To(MatchError(errPodNotInit))
})

// todo(rashmigottipati): add test to check VerifyPodRunning returning error
})
})
})
Loading