Ceph prometheus rules created by OCS operator instead of Rook #1615

travisn · 2022-04-04T23:06:54Z

Rook has stopped creating the prometheus rules with the cephcluster monitoring.enabled setting. Now the rules must be created separately from the cluster CR as described in the rook PR rook/rook#9837. The rules are fully owned downstream by the ocs operator now since upstream they are only installed by the helm chart. This also gives full flexibility downstream to update the rules only when QE determines we are ready for testing all the new rules.

At the same time, vendor for golang 1.17 to pick up prometheus. The prometheus api was not resolving unless requiring go 1.17 compatibility. Go 1.16 is confused by the moving around of the prometheus api between different packages from v0.43 to v0.46. Now the packages are updated for go 1.17 compatibility.

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=2066514

openshift-ci · 2022-04-04T23:09:43Z

@travisn: This pull request references Bugzilla bug 2066514, which is valid.

No validations were run on this bug

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (mbukatov@redhat.com), skipping review request.

In response to this:

Bug 2066514: Ceph prometheus rules created by OCS operator instead of Rook

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

travisn · 2022-04-04T23:10:42Z

@umangachapagain More testing is still needed, but please do let me know if there is any initial feedback, thanks.

controllers/storagecluster/cephcluster.go

Makefile

openshift-ci · 2022-04-05T03:46:05Z

@travisn: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

Ceph prometheus rules created by OCS operator instead of Rook

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

agarwal-mudit · 2022-04-05T03:46:48Z

No need to specify the bug in the title for main branch PRs.

Bug 2066514 can be mentioned in the comments if required.

go.mod

travisn · 2022-04-05T19:57:54Z

/retest

travisn · 2022-04-05T21:23:03Z

@umangachapagain Do you see an error in the CI? I'm not seeing the failure and it doesn't seem to be intermittent since rerunning gave the same results.

umangachapagain · 2022-04-06T06:22:23Z

controllers/storagecluster/cephcluster.go

+// state.
+func createPrometheusRules(r *StorageClusterReconciler, sc *ocsv1.StorageCluster, cluster *rookCephv1.CephCluster) error {
+	if !cluster.Spec.Monitoring.Enabled {
+		r.Log.Info("prometheus rules skipped", "CephCluster")


Suggested change

r.Log.Info("prometheus rules skipped", "CephCluster")

r.Log.Info("prometheus rules skipped", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

umangachapagain · 2022-04-06T06:22:40Z

controllers/storagecluster/cephcluster.go

+	}
+	prometheusRule, err := parsePrometheusRule(rules)
+	if err != nil {
+		r.Log.Error(err, "Unable to retrieve prometheus rules.", "CephCluster")


Suggested change

r.Log.Error(err, "Unable to retrieve prometheus rules.", "CephCluster")

r.Log.Error(err, "Unable to retrieve prometheus rules.", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

umangachapagain · 2022-04-06T06:23:55Z

controllers/storagecluster/cephcluster.go

+	applyLabels(getCephClusterMonitoringLabels(*sc), &prometheusRule.ObjectMeta)
+
+	if err := createOrUpdatePrometheusRule(r, sc, prometheusRule); err != nil {
+		r.Log.Error(err, "Prometheus rules could not be created.", "CephCluster")


Suggested change

r.Log.Error(err, "Prometheus rules could not be created.", "CephCluster")

r.Log.Error(err, "Prometheus rules could not be created.", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

umangachapagain · 2022-04-06T06:24:13Z

controllers/storagecluster/cephcluster.go

+		return err
+	}
+
+	r.Log.Info("prometheus rules deployed", "CephCluster")


Suggested change

r.Log.Info("prometheus rules deployed", "CephCluster")

r.Log.Info("prometheus rules deployed", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

umangachapagain · 2022-04-06T06:24:48Z

controllers/storagecluster/cephcluster.go

+	return nil
+}
+
+// OverwriteApplyToObjectMeta adds labels to object meta, overwriting keys that are already defined.


Suggested change

// OverwriteApplyToObjectMeta adds labels to object meta, overwriting keys that are already defined.

// applyLabels adds labels to object meta, overwriting keys that are already defined.

umangachapagain · 2022-04-06T06:25:34Z

controllers/storagecluster/cephcluster.go

+	}
+}
+
+// GetPrometheusRule returns provided prometheus rules or an error


Suggested change

// GetPrometheusRule returns provided prometheus rules or an error

// parsePrometheusRule returns provided prometheus rules or an error

umangachapagain · 2022-04-06T08:16:27Z

@umangachapagain Do you see an error in the CI? I'm not seeing the failure and it doesn't seem to be intermittent since rerunning gave the same results.

All required tests have passed. Remaining CI error is due to an issue with test scripts, not related to this PR.

Rook has stopped creating the prometheus rules with the cephcluster monitoring.enabled setting. Now the rules must be created separately from the cluster CR as described in the rook PR rook/rook#9837. The rules are fully owned downstream by the ocs operator now since upstream they are only installed by the helm chart. This also gives full flexibility downstream to update the rules only when QE determines we are ready for testing all the new rules. Signed-off-by: Travis Nielsen <tnielsen@redhat.com>

The prometheus api was not resolving unless requiring go 1.17 compatibility. Go 1.16 is confused by the moving around of the prometheus api between different packages from v0.43 to v0.46. Now the packages are updated for go 1.17 compatibility. Signed-off-by: Travis Nielsen <tnielsen@redhat.com>

The latest rook release is based on v1.9 instead of v1.8. Signed-off-by: Travis Nielsen <tnielsen@redhat.com>

openshift-ci · 2022-04-06T17:50:58Z

@travisn: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/red-hat-storage-ocs-ci-e2e-aws	`5668229`	link	false	`/test red-hat-storage-ocs-ci-e2e-aws`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

jarrpa

/lgtm

openshift-ci · 2022-04-12T18:12:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jarrpa, travisn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jarrpa]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

travisn changed the title ~~Ceph prometheus rules created by OCS operator instead of Rook~~ Bug 2066514: Ceph prometheus rules created by OCS operator instead of Rook Apr 4, 2022

openshift-ci bot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Apr 4, 2022

travisn commented Apr 4, 2022

View reviewed changes

controllers/storagecluster/cephcluster.go Show resolved Hide resolved

travisn commented Apr 4, 2022

View reviewed changes

Makefile Show resolved Hide resolved

agarwal-mudit changed the title ~~Bug 2066514: Ceph prometheus rules created by OCS operator instead of Rook~~ Ceph prometheus rules created by OCS operator instead of Rook Apr 5, 2022

openshift-ci bot removed the bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. label Apr 5, 2022

openshift-ci bot removed the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Apr 5, 2022

umangachapagain reviewed Apr 5, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

travisn force-pushed the rook-prometheus-rules branch 2 times, most recently from bcfef64 to 9ba2d2e Compare April 5, 2022 18:32

umangachapagain requested changes Apr 6, 2022

View reviewed changes

openshift-ci bot assigned umangachapagain Apr 6, 2022

travisn added 3 commits April 6, 2022 10:37

update to the rook v1.9.0-beta.0 release

5668229

The latest rook release is based on v1.9 instead of v1.8. Signed-off-by: Travis Nielsen <tnielsen@redhat.com>

travisn force-pushed the rook-prometheus-rules branch from 9ba2d2e to 5668229 Compare April 6, 2022 16:38

travisn requested a review from umangachapagain April 6, 2022 16:40

umangachapagain requested a review from jarrpa April 8, 2022 05:35

jarrpa approved these changes Apr 12, 2022

View reviewed changes

openshift-ci bot assigned jarrpa Apr 12, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 12, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2022

openshift-merge-robot merged commit 1a5f34d into red-hat-storage:main Apr 12, 2022

travisn deleted the rook-prometheus-rules branch April 12, 2022 20:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ceph prometheus rules created by OCS operator instead of Rook #1615

Ceph prometheus rules created by OCS operator instead of Rook #1615

travisn commented Apr 4, 2022 •

edited

openshift-ci bot commented Apr 4, 2022

travisn commented Apr 4, 2022

openshift-ci bot commented Apr 5, 2022

agarwal-mudit commented Apr 5, 2022

travisn commented Apr 5, 2022

travisn commented Apr 5, 2022

umangachapagain Apr 6, 2022

umangachapagain Apr 6, 2022

umangachapagain Apr 6, 2022

umangachapagain Apr 6, 2022

umangachapagain Apr 6, 2022

umangachapagain Apr 6, 2022

umangachapagain commented Apr 6, 2022

openshift-ci bot commented Apr 6, 2022

jarrpa left a comment

openshift-ci bot commented Apr 12, 2022

	r.Log.Info("prometheus rules skipped", "CephCluster")
	r.Log.Info("prometheus rules skipped", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

	r.Log.Error(err, "Unable to retrieve prometheus rules.", "CephCluster")
	r.Log.Error(err, "Unable to retrieve prometheus rules.", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

	r.Log.Error(err, "Prometheus rules could not be created.", "CephCluster")
	r.Log.Error(err, "Prometheus rules could not be created.", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

	r.Log.Info("prometheus rules deployed", "CephCluster")
	r.Log.Info("prometheus rules deployed", "CephCluster", klog.KRef(cluster.Namespace, cluster.Name))

	// OverwriteApplyToObjectMeta adds labels to object meta, overwriting keys that are already defined.
	// applyLabels adds labels to object meta, overwriting keys that are already defined.

	// GetPrometheusRule returns provided prometheus rules or an error
	// parsePrometheusRule returns provided prometheus rules or an error

Ceph prometheus rules created by OCS operator instead of Rook #1615

Ceph prometheus rules created by OCS operator instead of Rook #1615

Conversation

travisn commented Apr 4, 2022 • edited

openshift-ci bot commented Apr 4, 2022

travisn commented Apr 4, 2022

openshift-ci bot commented Apr 5, 2022

agarwal-mudit commented Apr 5, 2022

travisn commented Apr 5, 2022

travisn commented Apr 5, 2022

umangachapagain Apr 6, 2022

Choose a reason for hiding this comment

umangachapagain Apr 6, 2022

Choose a reason for hiding this comment

umangachapagain Apr 6, 2022

Choose a reason for hiding this comment

umangachapagain Apr 6, 2022

Choose a reason for hiding this comment

umangachapagain Apr 6, 2022

Choose a reason for hiding this comment

umangachapagain Apr 6, 2022

Choose a reason for hiding this comment

umangachapagain commented Apr 6, 2022

openshift-ci bot commented Apr 6, 2022

jarrpa left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Apr 12, 2022

travisn commented Apr 4, 2022 •

edited