Skip to content
Draft
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 @@ -41,8 +41,8 @@ func reconcileCatalogSource(cs *operatorsv1alpha1.CatalogSource, address string,
Priority: priority,
UpdateStrategy: &operatorsv1alpha1.UpdateStrategy{
RegistryPoll: &operatorsv1alpha1.RegistryPoll{
RawInterval: "10m",
Interval: &metav1.Duration{Duration: 10 * time.Minute},
RawInterval: "240m",
Interval: &metav1.Duration{Duration: 240 * time.Minute},
Comment on lines +44 to +45

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add unit tests for the interval change.

The coding guidelines require unit tests for code changes in control-plane-operator/**/*.go. While this is a configuration change, it significantly alters the polling behavior (from 10 minutes to 240 minutes). Consider adding a unit test that verifies both RawInterval and Interval fields are set to the expected 240-minute values in the reconciled CatalogSource.

🧪 Example test structure
func TestReconcileCatalogSourceRegistryPollInterval(t *testing.T) {
	cs := &operatorsv1alpha1.CatalogSource{}
	params := &OperatorLifecycleManagerParams{
		RedHatOperatorsImage: "test-image",
		OLMCatalogPlacement:  hyperv1.GuestOLMCatalogPlacement,
	}
	
	ReconcileRedHatOperatorsCatalogSource(cs, params)
	
	require.NotNil(t, cs.Spec.UpdateStrategy)
	require.NotNil(t, cs.Spec.UpdateStrategy.RegistryPoll)
	assert.Equal(t, "240m", cs.Spec.UpdateStrategy.RegistryPoll.RawInterval)
	assert.Equal(t, 240*time.Minute, cs.Spec.UpdateStrategy.RegistryPoll.Interval.Duration)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/olm/catalogs.go`
around lines 44 - 45, Add a unit test to verify the 240-minute polling interval
configuration change for the CatalogSource. Create a test function that
instantiates a CatalogSource, calls the function that modifies it (likely
ReconcileRedHatOperatorsCatalogSource based on the context), and then asserts
that both the RawInterval field equals "240m" and the Interval.Duration field
equals 240*time.Minute as shown in the provided example test structure. This
test should be placed in the appropriate test file for the catalogs.go module.

Source: Coding guidelines


⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

operator-framework api RegistryPoll RawInterval field format requirements version 0.37.0

💡 Result:

In the Operator Lifecycle Manager (OLM) API, the registry poll interval for a CatalogSource is configured via the interval field within the registryPoll updateStrategy [1][2]. This field expects a duration string format [3], such as "45m" or "10m" [1][2]. Regarding the RawInterval field: 1. Technical Role: The RawInterval field was introduced as an internal mechanism to handle the parsing of this interval [4][5]. It serves as a raw string representation of the duration before it is unmarshalled into the structured interval used by the OLM controller [4]. 2. Usage: This field is generally not intended for manual configuration by end-users in their YAML manifests [1][2]. Users should continue to use the interval field, which accepts standard duration strings [1][2]. 3. Version 0.37.0 context: The version 0.37.0 release of the operator-framework/api module refers to updates within the OLM API and associated ecosystem [6][7]. While this version includes updates for new fields (such as a new release field for CSVs) [6][7], the fundamental format requirements for catalog polling intervals remain based on standard duration parsing (e.g., Go's time.ParseDuration). In summary, users should specify the polling frequency using the interval field in their CatalogSource YAML using standard duration notation (e.g., 15m, 1h) [1][2]. The RawInterval field is an internal implementation detail and is not required or recommended for end-user manifests [4][5].

Citations:


🏁 Script executed:

#!/bin/bash
# Description: Search for RegistryPoll usage patterns and validation in the codebase

# Look for existing RegistryPoll configurations to confirm string format patterns
rg -n -C2 'RawInterval.*".*m"' --type=go

# Look for any validation or parsing of RawInterval
rg -n -C3 'RawInterval' --type=go -g '!vendor/' | head -50

Repository: openshift/hypershift

Length of output: 1592


Add unit tests for the catalog source polling interval changes.

The RawInterval string format "240m" is valid for Go duration parsing and is correct. However, this change modifies the polling behavior across all four default catalog sources (certified, community, marketplace, red-hat-operators) without accompanying unit tests. Control-plane-operator changes require unit tests per coding guidelines.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/olm/catalogs.go`
around lines 44 - 45, The polling interval changes for the four default catalog
sources (certified, community, marketplace, red-hat-operators) in the
catalogs.go file lack accompanying unit tests. Add unit tests that verify the
RawInterval and Interval fields are correctly configured for each of the four
default catalog sources with the 240-minute polling interval. The tests should
validate that the interval values are properly set and match across both the
string representation and the metav1.Duration object for each catalog source.

},
},
}
Expand Down