Repository: operator-framework/operator-sdk
Pull request #5015 (2021-06-26 19:35:14)

config3alphato3: Default domain to empty string
<!--

Welcome to the Operator SDK! Before contributing, make sure to:

- Read the contributing guidelines https://github.com/operator-framework/operator-sdk/blob/master/CONTRIBUTING.MD
- Rebase your branch on the latest upstream master
- Link any relevant issues, PR's, or documentation
- When fixing an issue, add "Closes #<ISSUE_NUMBER>"
- Follow the below checklist if making a user-facing change 

-->

**Description of the change:**

Update `convertConfig3AlphaTo3()` to use an empty string as a domain name
when the read config has no domain information.

Check the existence of "domain" field in the config before attempting a
type assertion. This helps avoid panic:

```
panic: interface conversion: interface {} is nil, not string
```

**Motivation for the change:**

I've an old project which was created with `3-alpha` PROJECT version a few months ago. It is based on an old operator-sdk project, before operator-sdk started using kubebuilder for project scaffold. The project's api version is of the format `group/v1`. All the new projects based on kubebuilder seem to use a domain name, resulting in the api version of the format `group.domain/v1`. Setting the domain to an empty string seems to work well with `kubebuilder/operator-sdk init --domain=""` to not break the CRD APIs of the old project. Found some tests in kubebuilder that ensures projects are valid without any domain.
When I tried to convert 3-alpha to 3 project, I got the following failure:

```console
$ operator-sdk alpha config-3alpha-to-3
WARN[0000] Config version 3-alpha has been stabilized as 3, and 3-alpha is no longer supported. Run `operator-sdk alpha config-3alpha-to-3` to upgrade your PROJECT config
 file to version 3
panic: interface conversion: interface {} is nil, not string

goroutine 1 [running]:
github.com/operator-framework/operator-sdk/internal/cmd/operator-sdk/alpha/config3alphato3.convertConfig3AlphaTo3(0xc00096c400, 0x123, 0x200, 0x1db5401, 0xc00057baf0, 0x0
, 0x0, 0x0)
        internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3.go:69 +0x171f
github.com/operator-framework/operator-sdk/internal/cmd/operator-sdk/alpha/config3alphato3.NewCmd.func1(0xc00094e000, 0x3538108, 0x0, 0x0, 0x0, 0x0)
        internal/cmd/operator-sdk/alpha/config3alphato3/cmd.go:48 +0x1b2
github.com/spf13/cobra.(*Command).execute(0xc00094e000, 0x3538108, 0x0, 0x0, 0xc00094e000, 0x3538108)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:850 +0x472
github.com/spf13/cobra.(*Command).ExecuteC(0xc00094e2c0, 0xc00094e2c0, 0x40d4aa, 0x7f006a977b00)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:958 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.1.1/command.go:895
sigs.k8s.io/kubebuilder/v3/pkg/cli.CLI.Run(...)
        /home/runner/go/pkg/mod/sigs.k8s.io/kubebuilder/v3@v3.0.0-alpha.0.0.20210518234629-191170994550/pkg/cli/cli.go:463
github.com/operator-framework/operator-sdk/internal/cmd/operator-sdk/cli.Run(0xc000182058, 0x41ad01)
        internal/cmd/operator-sdk/cli/cli.go:68 +0x65
main.main()
        cmd/operator-sdk/main.go:28 +0x25

```

Tried this with version v1.9.0 and a build from the master branch.

**Checklist**

If the pull request includes user-facing changes, extra documentation is required:
- [ ] Add a new changelog fragment in `changelog/fragments` (see [`changelog/fragments/00-template.yaml`](https://github.com/operator-framework/operator-sdk/tree/master/changelog/fragments/00-template.yaml))
- [ ] Add or update relevant sections of the docs website in [`website/content/en/docs`](https://github.com/operator-framework/operator-sdk/tree/master/website/content/en/docs)

I believe this is not a user-facing change, don't think the above check boxes need to be ticked.


Patch:
diff --git a/changelog/fragments/generate-bundle-fix.yaml b/changelog/fragments/generate-bundle-fix.yaml
new file mode 100644
index 00000000..88320363
--- /dev/null
+++ b/changelog/fragments/generate-bundle-fix.yaml
@@ -0,0 +1,17 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      Fixed the `operator-sdk generate bundle` command to specify the right path of bundle
+      metadata in bundle.Dcokerfile.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: bugfix
+
+    # Is this a breaking change?
+    breaking: false
diff --git a/changelog/fragments/helm-deepequals.yaml b/changelog/fragments/helm-deepequals.yaml
new file mode 100644
index 00000000..0a331945
--- /dev/null
+++ b/changelog/fragments/helm-deepequals.yaml
@@ -0,0 +1,4 @@
+entries:
+  - description: >
+      For Helm-based operators, fixed release equality comparison such that number values are compared and not their types to avoid unnecessary reconciliations.
+    kind: bugfix
\ No newline at end of file
diff --git a/changelog/fragments/java-operator-bump.yaml b/changelog/fragments/java-operator-bump.yaml
new file mode 100644
index 00000000..83cb884d
--- /dev/null
+++ b/changelog/fragments/java-operator-bump.yaml
@@ -0,0 +1,16 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      Bump java-operator-plugins. Adds `namespaced` plugin attribute and fixes resource version generation.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: "bugfix"
+
+    # Is this a breaking change?
+    breaking: false
diff --git a/go.mod b/go.mod
index bb716d79..abcd8c14 100644
--- a/go.mod
+++ b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/onsi/ginkgo v1.15.2
 	github.com/onsi/gomega v1.11.0
 	github.com/operator-framework/api v0.8.2-0.20210526151024-41d37db9141f
-	github.com/operator-framework/java-operator-plugins v0.0.0-20210525141944-8303c38a876d
+	github.com/operator-framework/java-operator-plugins v0.0.0-20210629174633-88afbd6d98b2
 	github.com/operator-framework/operator-lib v0.4.1
 	github.com/operator-framework/operator-registry v1.15.3
 	github.com/prometheus/client_golang v1.7.1
diff --git a/go.sum b/go.sum
index b95a9a79..cef87ad7 100644
--- a/go.sum
+++ b/go.sum
@@ -770,8 +770,8 @@ github.com/operator-framework/api v0.3.22/go.mod h1:GVNiB6AQucwdZz3ZFXNv9HtcLOzc
 github.com/operator-framework/api v0.5.2/go.mod h1:L7IvLd/ckxJEJg/t4oTTlnHKAJIP/p51AvEslW3wYdY=
 github.com/operator-framework/api v0.8.2-0.20210526151024-41d37db9141f h1:VX19pxX9NNJNoh0kfS0PSzK4lOInXh4+4VNlbaED/gg=
 github.com/operator-framework/api v0.8.2-0.20210526151024-41d37db9141f/go.mod h1:L7IvLd/ckxJEJg/t4oTTlnHKAJIP/p51AvEslW3wYdY=
-github.com/operator-framework/java-operator-plugins v0.0.0-20210525141944-8303c38a876d h1:HE4CQY27vaQWQnk7oD2mku417tKyOz6UQHOUNVCuFHU=
-github.com/operator-framework/java-operator-plugins v0.0.0-20210525141944-8303c38a876d/go.mod h1:QFcG224Nx8VMwBEqxpZbT17LkISNzlOjY1/r4OQBYic=
+github.com/operator-framework/java-operator-plugins v0.0.0-20210629174633-88afbd6d98b2 h1:BA0zuCEz9wNTc582KTz0hczk6pwI/fQWaYZlQ5eCAjg=
+github.com/operator-framework/java-operator-plugins v0.0.0-20210629174633-88afbd6d98b2/go.mod h1:QFcG224Nx8VMwBEqxpZbT17LkISNzlOjY1/r4OQBYic=
 github.com/operator-framework/operator-lib v0.4.1 h1:Eh4JHs+LAWeC85ZMHXJ9RXg7G5grYx8J29TkOw8003s=
 github.com/operator-framework/operator-lib v0.4.1/go.mod h1:2dszbSeSo/472Ea8zIAcjEJhgzXKxzAGG24MgIP+13c=
 github.com/operator-framework/operator-registry v1.15.3 h1:C+u+zjDh6yQAKN+DbUvPeLjojZtJftvp/J28rRqiWWU=
diff --git a/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3.go b/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3.go
index fd19c6cf..2a5a41b8 100644
--- a/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3.go
+++ b/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3.go
@@ -66,7 +66,12 @@ func convertConfig3AlphaTo3(cfgBytes []byte) (_ []byte, err error) {
 	isMultigroup := hasMultigroup && multigroupObj.(bool)
 
 	if obj, hasRes := cfgObj["resources"]; hasRes {
-		domain := cfgObj["domain"].(string)
+		// Default to empty domain if no domain information is found.
+		domain := ""
+		if domainObj, ok := cfgObj["domain"]; ok {
+			domain = domainObj.(string)
+		}
+
 		resObjs := obj.([]interface{})
 		resources := make([]resource.Resource, len(resObjs))
 
diff --git a/internal/helm/release/manager.go b/internal/helm/release/manager.go
index 7ee19a0b..ffeb40b2 100644
--- a/internal/helm/release/manager.go
+++ b/internal/helm/release/manager.go
@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"reflect"
 	"strings"
 
 	jsonpatch "gomodules.xyz/jsonpatch/v3"
@@ -126,8 +127,10 @@ func (m *manager) Sync(ctx context.Context) error {
 	m.deployedRelease = deployedRelease
 	m.isInstalled = true
 
-	m.isUpgradeRequired = m.isUpgrade(deployedRelease)
-
+	m.isUpgradeRequired, err = m.isUpgrade(deployedRelease)
+	if err != nil {
+		return fmt.Errorf("failed to get upgrade status: %w", err)
+	}
 	return nil
 }
 
@@ -135,18 +138,41 @@ func notFoundErr(err error) bool {
 	return err != nil && strings.Contains(err.Error(), "not found")
 }
 
-func (m manager) isUpgrade(deployedRelease *rpb.Release) bool {
+// This is caused by the different logic of loading from local and loading from secret
+// For example, the Raw field, which has the tag `json:"-"`, causes the Unmarshal to be lost when it into Release
+// We need to make them follow the JSON tag
+// see: https://github.com/helm/helm/blob/cf0c6fed519d48101cd69ce01a355125215ee46f/pkg/storage/driver/util.go#L81
+func equalJSONStruct(a, b interface{}) (bool, error) {
+	if reflect.ValueOf(a).IsNil() || reflect.ValueOf(b).IsNil() {
+		return apiequality.Semantic.DeepEqual(a, b), nil
+	}
+
+	aBuf, bBuf := &bytes.Buffer{}, &bytes.Buffer{}
+	err := json.NewEncoder(aBuf).Encode(a)
+	if err != nil {
+		return false, err
+	}
+	err = json.NewEncoder(bBuf).Encode(b)
+	return aBuf.String() == bBuf.String(), err
+}
+
+func (m manager) isUpgrade(deployedRelease *rpb.Release) (bool, error) {
 	if deployedRelease == nil {
-		return false
+		return false, nil
 	}
 
 	// Judging whether to skip updates
 	skip := m.namespace == deployedRelease.Namespace
 	skip = skip && m.releaseName == deployedRelease.Name
-	skip = skip && apiequality.Semantic.DeepEqual(m.chart, deployedRelease.Chart)
-	skip = skip && apiequality.Semantic.DeepEqual(m.values, deployedRelease.Config)
 
-	return !skip
+	ok, err := equalJSONStruct(m.chart, deployedRelease.Chart)
+	if err != nil {
+		return false, err
+	}
+	skip = skip && ok
+
+	ok, err = equalJSONStruct(m.values, deployedRelease.Config)
+	return !(skip && ok), err
 }
 
 func (m manager) getDeployedRelease() (*rpb.Release, error) {
diff --git a/internal/util/bundleutil/bundleutil.go b/internal/util/bundleutil/bundleutil.go
index eab231cf..a604c3c1 100644
--- a/internal/util/bundleutil/bundleutil.go
+++ b/internal/util/bundleutil/bundleutil.go
@@ -88,7 +88,7 @@ func (meta *BundleMetaData) GenerateMetadata() error {
 	// Create annotation values for both bundle.Dockerfile and annotations.yaml, which should
 	// hold the same set of values always.
 	values := annotationsValues{
-		BundleDir:                filepath.Base(meta.BundleDir),
+		BundleDir:                meta.BundleDir,
 		PackageName:              meta.PackageName,
 		Channels:                 meta.Channels,
 		DefaultChannel:           meta.DefaultChannel,
@@ -107,11 +107,13 @@ func (meta *BundleMetaData) GenerateMetadata() error {
 	}
 
 	dockerfilePath := defaultBundleDockerfilePath
-	// If migrating from packagemanifests to bundle, bundle.Docker file is present
-	// inside bundleDir, else its in the project directory.
+	// If migrating from packagemanifests to bundle, bundle.Dockerfile is present
+	// inside bundleDir, else its in the project directory. Hence dockerfile
+	// should have the path specified with respect to output directory of resulting bundles.
 	// Remmove this, when pkgman-to-bundle migrate command is removed.
 	if len(meta.PkgmanifestPath) != 0 {
 		dockerfilePath = filepath.Join(filepath.Dir(meta.BundleDir), "bundle.Dockerfile")
+		values.BundleDir = filepath.Base(meta.BundleDir)
 	}
 
 	templateMap := map[string]*template.Template{
diff --git a/website/content/en/docs/building-operators/ansible/reference/internal_metrics.md b/website/content/en/docs/building-operators/ansible/reference/internal_metrics.md
new file mode 100644
index 00000000..1a7377ea
--- /dev/null
+++ b/website/content/en/docs/building-operators/ansible/reference/internal_metrics.md
@@ -0,0 +1,16 @@
+The Ansible Operator comes with three internal metrics that provide an insight to the frequency and time of operator reconciliations. These metrics can be
+scraped by a Prometheus instance or any other openmetrics system. To publish operator metrics and scrape them with an openmetrics system such as Prometheus, view 
+[Kubebuilder documentation](https://book.kubebuilder.io/reference/metrics.html) on publishing metrics.
+
+The default metrics recorded in Operator SDK are collected in a [histogram](https://prometheus.io/docs/practices/histograms/).
+
+The following three metrics are derived from the histogram:
+1. `ansible_operator_reconciles_bucket` - Each bucket in the histogram counts the number of reconciliations that have a period (in seconds) less than or equal
+to the upper limit of the bucket.
+3. `ansible_operator_reconciles_count` - The total number of reconciliations that have occured up to that instance of time while running an Ansible operator.
+4. `ansible_operator_reconciles_sum` - The cumulative amount of time (in seconds) of all reconciliations that have occured up to that instance of time while 
+running an Ansible operator.
+
+These metrics can be queried in the Prometheus UI.
+
+![Screen Shot 2021-06-24 at 2 10 28 PM](https://user-images.githubusercontent.com/37827279/123332879-f0fb2900-d4f5-11eb-87ea-7afd04f35b1c.png)


Test patch:
diff --git a/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3_test.go b/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3_test.go
index b8794ba5..00dd5de0 100644
--- a/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3_test.go
+++ b/internal/cmd/operator-sdk/alpha/config3alphato3/convert_config_3-alpha_to_3_test.go
@@ -36,6 +36,7 @@ var _ = Describe("ConvertConfig3AlphaTo3", func() {
 		Entry("no resources", noResourcesConfig, noResourcesConfigExp),
 		Entry("basic", basicConfig, basicConfigExp),
 		Entry("complex", complexConfig, complexConfigExp),
+		Entry("no domain", noDomainConfig, noDomainConfigExp),
 	)
 })
 
@@ -68,6 +69,30 @@ version: "3"
 `
 )
 
+const (
+	noDomainConfig = `layout: ansible.sdk.operatorframework.io/v1
+projectName: memcached-operator
+resources:
+- crdVersion: v1
+  group: cache
+  kind: Memcached
+  version: v1alpha1
+version: 3-alpha`
+	noDomainConfigExp = `layout: ansible.sdk.operatorframework.io/v1
+projectName: memcached-operator
+resources:
+- api:
+    crdVersion: v1
+    # TODO(user): Uncomment the below line if this resource's CRD is namespace scoped, else delete it.
+    # namespaced: true
+  # TODO(user): Uncomment the below line if this resource implements a controller, else delete it.
+  # controller: true
+  group: cache
+  kind: Memcached
+  version: v1alpha1
+version: "3"`
+)
+
 const (
 	noResourcesConfig = `domain: example.com
 layout: ansible.sdk.operatorframework.io/v1
diff --git a/internal/helm/release/manager_test.go b/internal/helm/release/manager_test.go
index f930374e..70c5e5b5 100644
--- a/internal/helm/release/manager_test.go
+++ b/internal/helm/release/manager_test.go
@@ -15,6 +15,8 @@
 package release
 
 import (
+	"bytes"
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -247,11 +249,20 @@ func TestManagerisUpgrade(t *testing.T) {
 			name:            "different values",
 			releaseName:     "deployed",
 			releaseNs:       "deployed-ns",
-			values:          map[string]interface{}{"key": "1"},
+			values:          map[string]interface{}{"key": "1", "int": int32(1)},
 			chart:           newTestChart(t, "./testdata/simple"),
-			deployedRelease: newTestRelease(newTestChart(t, "./testdata/simple"), map[string]interface{}{"key": ""}, "deployed", "deployed-ns"),
+			deployedRelease: newTestRelease(newTestChart(t, "./testdata/simple"), map[string]interface{}{"key": "", "int": int64(1)}, "deployed", "deployed-ns"),
 			want:            true,
 		},
+		{
+			name:            "nil values",
+			releaseName:     "deployed",
+			releaseNs:       "deployed-ns",
+			values:          nil,
+			chart:           newTestChart(t, "./testdata/simple"),
+			deployedRelease: newTestRelease(newTestChart(t, "./testdata/simple"), map[string]interface{}{}, "deployed", "deployed-ns"),
+			want:            false,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
@@ -261,8 +272,9 @@ func TestManagerisUpgrade(t *testing.T) {
 				values:      test.values,
 				chart:       test.chart,
 			}
-			isUpgrade := m.isUpgrade(test.deployedRelease)
+			isUpgrade, err := m.isUpgrade(test.deployedRelease)
 			assert.Equal(t, test.want, isUpgrade)
+			assert.Equal(t, nil, err)
 		})
 	}
 }
@@ -273,13 +285,16 @@ func newTestChart(t *testing.T, path string) *cpb.Chart {
 	return chart
 }
 
-func newTestRelease(chart *cpb.Chart, values map[string]interface{}, name, namespace string) *rpb.Release {
+func newTestRelease(chart *cpb.Chart, values map[string]interface{}, name, namespace string) *rpb.Release { // nolint: unparam
 	release := rpb.Mock(&rpb.MockReleaseOptions{
 		Name:      name,
 		Namespace: namespace,
-		Chart:     chart,
 		Version:   1,
 	})
+
+	buffer := &bytes.Buffer{}
+	_ = json.NewEncoder(buffer).Encode(chart)
+	_ = json.NewDecoder(buffer).Decode(release.Chart)
 	release.Config = values
 	return release
 }
diff --git a/website/content/en/docs/olm-integration/testing-deployment.md b/website/content/en/docs/olm-integration/testing-deployment.md
index 3248dcb4..d69eab15 100644
--- a/website/content/en/docs/olm-integration/testing-deployment.md
+++ b/website/content/en/docs/olm-integration/testing-deployment.md
@@ -176,3 +176,4 @@ Let's look at the anatomy of the `cleanup` configuration model:
 [cli-olm-status]:/docs/cli/operator-sdk_olm_status
 [creating-bundles]:/docs/olm-integration/quickstart-bundle/#creating-a-bundle
 [add-sa-secret]:https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#add-imagepullsecrets-to-a-service-account
+[image-reg-config]:/docs/olm-integration/cli-overview#private-bundle-and-catalog-image-registries