Skip to content

Commit 29bc67d

Browse files
hongkailiuhongkailiu
committed
Define constants for external usages
`github.com/openshift/cluster-version-operator/pkg/external/constants.go` seems the authentic place for the constants, but it is private to the packages in the same root [1]. This change introduces shortcuts to the constants that are used externally. [1]. https://pkg.go.dev/cmd/go#hdr-Internal_Directories
1 parent a4b7547 commit 29bc67d

8 files changed

Lines changed: 51 additions & 28 deletions

File tree

‎lib/resourcebuilder/apps.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1414
"k8s.io/klog/v2"
1515

16+
"github.com/openshift/cluster-version-operator/pkg/external"
1617
"github.com/openshift/cluster-version-operator/pkg/payload"
1718
)
1819

@@ -39,7 +40,7 @@ func (b *builder) modifyDeployment(ctx context.Context, deployment *appsv1.Deplo
3940
// if we detect the CVO deployment we need to replace the KUBERNETES_SERVICE_HOST env var with the internal load
4041
// balancer to be resilient to kube-apiserver rollouts that cause the localhost server to become non-responsive for
4142
// multiple minutes.
42-
if deployment.Namespace == "openshift-cluster-version" && deployment.Name == "cluster-version-operator" {
43+
if deployment.Namespace == external.DefaultCVONamespace && deployment.Name == external.DefaultDeploymentName {
4344
infrastructureConfig, err := b.configClientv1.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{})
4445
// not found just means that we don't have infrastructure configuration yet, so we should tolerate not found and avoid substitution
4546
if err != nil && !errors.IsNotFound(err) {
@@ -63,7 +64,7 @@ func (b *builder) modifyDeployment(ctx context.Context, deployment *appsv1.Deplo
6364
}
6465
err = updatePodSpecWithInternalLoadBalancerKubeService(
6566
&deployment.Spec.Template.Spec,
66-
[]string{"cluster-version-operator"},
67+
[]string{external.DefaultContainerName},
6768
lbHost,
6869
lbPort,
6970
)

‎pkg/cvo/sync_worker.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
configv1 "github.com/openshift/api/config/v1"
2727

2828
"github.com/openshift/cluster-version-operator/lib/capability"
29+
"github.com/openshift/cluster-version-operator/pkg/internal"
2930
"github.com/openshift/cluster-version-operator/pkg/payload"
3031
"github.com/openshift/cluster-version-operator/pkg/payload/precondition"
3132
)
@@ -329,7 +330,7 @@ func (w *SyncWorker) syncPayload(ctx context.Context, work *SyncWork) ([]configv
329330
// The remainder of this logic is for loading a new payload.
330331
// Any filtering as to not needing to reload the payload should be done in the switch before this point.
331332

332-
cvoObjectRef := &corev1.ObjectReference{APIVersion: "config.openshift.io/v1", Kind: "ClusterVersion", Name: "version", Namespace: "openshift-cluster-version"}
333+
cvoObjectRef := &corev1.ObjectReference{APIVersion: "config.openshift.io/v1", Kind: "ClusterVersion", Name: internal.DefaultClusterVersionName, Namespace: internal.DefaultCVONamespace}
333334
msg := fmt.Sprintf("Retrieving and verifying payload version=%q image=%q", desired.Version, desired.Image)
334335
w.eventRecorder.Eventf(cvoObjectRef, corev1.EventTypeNormal, "RetrievePayload", msg)
335336
reporter.ReportPayload(LoadPayloadStatus{

‎pkg/external/constants.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package external
2+
3+
import "github.com/openshift/cluster-version-operator/pkg/internal"
4+
5+
// The constants defined here are used by the components, e.g., e2e tests that have no access
6+
// to github.com/openshift/cluster-version-operator/pkg/internal
7+
// See https://pkg.go.dev/cmd/go#hdr-Internal_Directories
8+
const (
9+
// DefaultCVONamespace is the default namespace for the Cluster Version Operator
10+
DefaultCVONamespace = internal.DefaultCVONamespace
11+
// DefaultClusterVersionName is the default name for the Cluster Version resource managed by the Cluster Version Operator
12+
DefaultClusterVersionName = internal.DefaultClusterVersionName
13+
// DefaultDeploymentName is the default name of the deployment for the Cluster Version Operator
14+
DefaultDeploymentName = internal.DefaultDeploymentName
15+
// DefaultContainerName is the default container name in the deployment for the Cluster Version Operator
16+
DefaultContainerName = internal.DefaultContainerName
17+
18+
// ConditionalUpdateConditionTypeRecommended is a type of the condition present on a conditional update
19+
// that indicates whether the conditional update is recommended or not
20+
ConditionalUpdateConditionTypeRecommended = internal.ConditionalUpdateConditionTypeRecommended
21+
)

‎pkg/internal/constants.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ import (
77
)
88

99
const (
10+
// DefaultCVONamespace is the default namespace for the Cluster Version Operator
11+
DefaultCVONamespace = "openshift-cluster-version"
12+
// DefaultClusterVersionName is the default name for the Cluster Version resource managed by the Cluster Version Operator
13+
DefaultClusterVersionName = "version"
14+
// DefaultDeploymentName is the default name of the deployment for the Cluster Version Operator
15+
DefaultDeploymentName = "cluster-version-operator"
16+
// DefaultContainerName is the default container name in the deployment for the Cluster Version Operator
17+
DefaultContainerName = DefaultDeploymentName
18+
1019
ConfigNamespace = "openshift-config"
1120
ConfigManagedNamespace = "openshift-config-managed"
1221
AdminGatesConfigMap = "admin-gates"

‎pkg/start/start.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ import (
4949
)
5050

5151
const (
52-
defaultComponentName = "version"
53-
defaultComponentNamespace = "openshift-cluster-version"
54-
5552
minResyncPeriod = 2 * time.Minute
5653
)
5754

@@ -122,8 +119,8 @@ func NewOptions() *Options {
122119
PromQLTarget: defaultPromQLTarget,
123120

124121
// exposed only for testing
125-
Namespace: defaultEnv("CVO_NAMESPACE", defaultComponentNamespace),
126-
Name: defaultEnv("CVO_NAME", defaultComponentName),
122+
Namespace: defaultEnv("CVO_NAMESPACE", internal.DefaultCVONamespace),
123+
Name: defaultEnv("CVO_NAME", internal.DefaultClusterVersionName),
127124
PayloadOverride: os.Getenv("PAYLOAD_OVERRIDE"),
128125
ResyncInterval: minResyncPeriod,
129126
Exclude: os.Getenv("EXCLUDE_MANIFESTS"),

‎test/cvo/accept_risks.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"k8s.io/apimachinery/pkg/util/wait"
1515
"k8s.io/client-go/rest"
1616

17+
"github.com/openshift/cluster-version-operator/pkg/external"
1718
"github.com/openshift/cluster-version-operator/test/util"
1819
)
1920

@@ -38,21 +39,21 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`,
3839
o.Expect(util.SkipIfHypershift(ctx, c)).To(o.BeNil())
3940
o.Expect(util.SkipIfMicroshift(ctx, c)).To(o.BeNil())
4041

41-
cv, err := configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
42+
cv, err := configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{})
4243
backup = *cv.Spec.DeepCopy()
4344
o.Expect(err).NotTo(o.HaveOccurred())
4445
})
4546

4647
g.AfterEach(func() {
47-
cv, err := configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
48+
cv, err := configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{})
4849
o.Expect(err).NotTo(o.HaveOccurred())
4950
cv.Spec = backup
5051
_, err = configClient.ClusterVersions().Update(ctx, cv, metav1.UpdateOptions{})
5152
o.Expect(err).NotTo(o.HaveOccurred())
5253
})
5354

5455
g.It("should work with accept risks [Serial]", func() {
55-
cv, err := configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
56+
cv, err := configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{})
5657
o.Expect(err).NotTo(o.HaveOccurred())
5758

5859
g.By("Using fauxinnati as the upstream and its risks-always channel")
@@ -65,7 +66,7 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`,
6566
g.By("Checking that conditional updates shows up in status")
6667
// waiting for the conditional updates to show up
6768
o.Expect(wait.PollUntilContextTimeout(ctx, 30*time.Second, 5*time.Minute, true, func(ctx context.Context) (done bool, err error) {
68-
cv, err = configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
69+
cv, err = configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{})
6970
o.Expect(err).NotTo(o.HaveOccurred())
7071
if len(cv.Status.ConditionalUpdates) == 0 {
7172
return false, nil
@@ -81,7 +82,7 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`,
8182
acceptRisks = append(acceptRisks, configv1.AcceptRisk{Name: name})
8283
}
8384
o.Expect(cu.Risks).NotTo(o.BeEmpty())
84-
recommendedCondition := meta.FindStatusCondition(cu.Conditions, conditionalUpdateConditionTypeRecommended)
85+
recommendedCondition := meta.FindStatusCondition(cu.Conditions, external.ConditionalUpdateConditionTypeRecommended)
8586
o.Expect(recommendedCondition).NotTo(o.BeNil())
8687
o.Expect(recommendedCondition.Status).To(o.Equal(metav1.ConditionFalse))
8788
}
@@ -102,11 +103,11 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`,
102103
g.By("Checking that all conditional updates are recommended")
103104
// waiting for the conditional updates to show up
104105
o.Expect(wait.PollUntilContextTimeout(ctx, 30*time.Second, 5*time.Minute, true, func(ctx context.Context) (done bool, err error) {
105-
cv, err = configClient.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
106+
cv, err = configClient.ClusterVersions().Get(ctx, external.DefaultClusterVersionName, metav1.GetOptions{})
106107
o.Expect(err).NotTo(o.HaveOccurred())
107108
o.Expect(cv.Status.ConditionalUpdates).NotTo(o.BeEmpty())
108109
for _, cu := range cv.Status.ConditionalUpdates {
109-
recommendedCondition := meta.FindStatusCondition(cu.Conditions, conditionalUpdateConditionTypeRecommended)
110+
recommendedCondition := meta.FindStatusCondition(cu.Conditions, external.ConditionalUpdateConditionTypeRecommended)
110111
o.Expect(recommendedCondition).NotTo(o.BeNil())
111112
if recommendedCondition.Status != metav1.ConditionTrue {
112113
return false, nil

‎test/cvo/constants.go‎

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎test/cvo/cvo.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
g "github.com/onsi/ginkgo/v2"
1111
o "github.com/onsi/gomega"
1212

13+
"github.com/openshift/cluster-version-operator/pkg/external"
1314
"github.com/openshift/cluster-version-operator/test/oc"
1415
ocapi "github.com/openshift/cluster-version-operator/test/oc/api"
1516
"github.com/openshift/cluster-version-operator/test/util"
@@ -64,14 +65,14 @@ var _ = g.Describe(`[Jira:"Cluster Version Operator"] cluster-version-operator`,
6465
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to determine if cluster is MicroShift")
6566

6667
g.By("Checking that the 'openshift.io/run-level' label exists on the namespace and has the empty value")
67-
ns, err := kubeClient.CoreV1().Namespaces().Get(ctx, cvoNamespace, metav1.GetOptions{})
68-
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get namespace %s", cvoNamespace)
68+
ns, err := kubeClient.CoreV1().Namespaces().Get(ctx, external.DefaultCVONamespace, metav1.GetOptions{})
69+
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to get namespace %s", external.DefaultCVONamespace)
6970
runLevel, exists := ns.Labels["openshift.io/run-level"]
70-
o.Expect(exists).To(o.BeTrue(), "The 'openshift.io/run-level' label on namespace %s does not exist", cvoNamespace)
71-
o.Expect(runLevel).To(o.BeEmpty(), "Expected the 'openshift.io/run-level' label value on namespace %s has the empty value, but got %s", cvoNamespace, runLevel)
71+
o.Expect(exists).To(o.BeTrue(), "The 'openshift.io/run-level' label on namespace %s does not exist", external.DefaultCVONamespace)
72+
o.Expect(runLevel).To(o.BeEmpty(), "Expected the 'openshift.io/run-level' label value on namespace %s has the empty value, but got %s", external.DefaultCVONamespace, runLevel)
7273

7374
g.By("Checking that the annotation 'openshift.io/scc annotation' on the CVO pod has the value hostaccess")
74-
podList, err := kubeClient.CoreV1().Pods(cvoNamespace).List(ctx, metav1.ListOptions{
75+
podList, err := kubeClient.CoreV1().Pods(external.DefaultCVONamespace).List(ctx, metav1.ListOptions{
7576
LabelSelector: "k8s-app=cluster-version-operator",
7677
FieldSelector: "status.phase=Running",
7778
})

0 commit comments

Comments
 (0)