Skip to content
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

Jump to bottom

Don't use the ToolchainCluster.Spec fields #1054

Merged
merged 4 commits into from
Oct 4, 2024
Merged

Don't use the ToolchainCluster.Spec fields #1054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2e83d9b
Don't use the deprecated fields in ToolchainCluster.
metlos Sep 18, 2024
374daa0
Merge remote-tracking branch 'upstream/master' into deprecate-tc-spec…
metlos Oct 1, 2024
1ade6a6
Update the comment to not mention Spec.APIEndpoint
metlos Oct 2, 2024
3389b7d
Remove the remnants of checking for the namespace label.
metlos Oct 3, 2024
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
6 changes: 2 additions & 4 deletions setup/users/create_users.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,11 @@ func getMemberClusterName(cl client.Client, hostOperatorNamespace, memberOperato
var memberCluster toolchainv1alpha1.ToolchainCluster
err := k8swait.Poll(configuration.DefaultRetryInterval, configuration.DefaultTimeout, func() (bool, error) {
clusters := &toolchainv1alpha1.ToolchainClusterList{}
if err := cl.List(context.TODO(), clusters, client.InNamespace(hostOperatorNamespace), client.MatchingLabels{
"namespace": memberOperatorNamespace,
}); err != nil {
if err := cl.List(context.TODO(), clusters, client.InNamespace(hostOperatorNamespace)); err != nil {
return false, err
}
for _, cluster := range clusters.Items {
if condition.IsTrue(cluster.Status.Conditions, toolchainv1alpha1.ConditionReady) {
if cluster.Status.OperatorNamespace == memberOperatorNamespace && condition.IsTrue(cluster.Status.Conditions, toolchainv1alpha1.ConditionReady) {
memberCluster = cluster
return true, nil
}
Expand Down
4 changes: 1 addition & 3 deletions setup/users/create_users_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@ func TestCreate(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Namespace: hostOperatorNamespace,
Name: "member-abcd",
Labels: map[string]string{
"namespace": memberOperatorNamespace,
},
},
Status: toolchainv1alpha1.ToolchainClusterStatus{
OperatorNamespace: memberOperatorNamespace,
Conditions: []toolchainv1alpha1.Condition{
{
Type: toolchainv1alpha1.ConditionReady,
Expand Down
5 changes: 2 additions & 3 deletions test/e2e/parallel/registration_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func TestLandingPageReachable(t *testing.T) {
}

func TestRegistrationServiceMetricsEndpoint(t *testing.T) {

// given
await := WaitForDeployments(t)
t.Parallel()
Expand Down Expand Up @@ -108,7 +107,6 @@ func TestRegistrationServiceMetricsEndpoint(t *testing.T) {
assert.Contains(t, string(samples), "sandbox_promhttp_request_duration_seconds_sum")
assert.Contains(t, string(samples), "sandbox_promhttp_request_duration_seconds_count")
})

}

func TestHealth(t *testing.T) {
Expand Down Expand Up @@ -463,6 +461,7 @@ func TestSignupOK(t *testing.T) {
signupUser(token, emailAddress, identity.Username, identity)
})
}

func TestUserSignupFoundWhenNamedWithEncodedUsername(t *testing.T) {
// given
t.Parallel()
Expand Down Expand Up @@ -960,7 +959,7 @@ func signupIsProvisioned(client *GetSignupClient) {
transformedUsername := commonsignup.TransformUsername(client.username, []string{"openshift", "kube", "default", "redhat", "sandbox"}, []string{"admin"})
require.NoError(client.t, err)
require.True(client.t, found)
assert.Equal(client.t, memberCluster.Spec.APIEndpoint, client.responseBody["apiEndpoint"])
assert.Equal(client.t, memberCluster.Status.APIEndpoint, client.responseBody["apiEndpoint"])
assert.Equal(client.t, hostAwait.APIProxyURL, client.responseBody["proxyURL"])
assert.Equal(client.t, fmt.Sprintf("%s-dev", transformedUsername), client.responseBody["defaultUserNamespace"])
assertRHODSClusterURL(client.t, memberAwait, client.responseBody)
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/parallel/spacerequest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestCreateSpaceRequest(t *testing.T) {
subSpace, _ = VerifyResourcesProvisionedForSpace(t, awaitilities, subSpace.Name, UntilSpaceHasAnyTargetClusterSet())
spaceRequest, err = memberAwait.WaitForSpaceRequest(t, types.NamespacedName{Namespace: spaceRequest.GetNamespace(), Name: spaceRequest.GetName()},
UntilSpaceRequestHasConditions(Provisioned()),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Spec.APIEndpoint),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Status.APIEndpoint),
UntilSpaceRequestHasNamespaceAccess(subSpace),
)
require.NoError(t, err)
Expand Down Expand Up @@ -138,7 +138,7 @@ func TestCreateSpaceRequest(t *testing.T) {
subSpace, _ = VerifyResourcesProvisionedForSpace(t, awaitilities, subSpace.Name, UntilSpaceHasAnyTargetClusterSet())
spaceRequest, err = memberAwait.WaitForSpaceRequest(t, types.NamespacedName{Namespace: spaceRequest.GetNamespace(), Name: spaceRequest.GetName()},
UntilSpaceRequestHasConditions(Provisioned()),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Spec.APIEndpoint),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Status.APIEndpoint),
UntilSpaceRequestHasNamespaceAccess(subSpace),
UntilSpaceRequestHasDisableInheritance(true),
)
Expand All @@ -165,7 +165,7 @@ func TestCreateSpaceRequest(t *testing.T) {
subSpace, _ = VerifyResourcesProvisionedForSpace(t, awaitilities, subSpace.Name, UntilSpaceHasAnyTargetClusterSet())
spaceRequest, err = memberAwait.WaitForSpaceRequest(t, types.NamespacedName{Namespace: spaceRequest.GetNamespace(), Name: spaceRequest.GetName()},
UntilSpaceRequestHasConditions(Provisioned()),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Spec.APIEndpoint),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Status.APIEndpoint),
UntilSpaceRequestHasNamespaceAccess(subSpace))
require.NoError(t, err)
VerifyNamespaceAccessForSpaceRequest(t, memberAwait.Client, spaceRequest)
Expand Down Expand Up @@ -221,7 +221,7 @@ func TestCreateSpaceRequest(t *testing.T) {
subSpace, _ = VerifyResourcesProvisionedForSpace(t, awaitilities, subSpace.Name, UntilSpaceHasAnyTargetClusterSet())
spaceRequest, err = memberAwait.WaitForSpaceRequest(t, types.NamespacedName{Namespace: spaceRequest.GetNamespace(), Name: spaceRequest.GetName()},
UntilSpaceRequestHasConditions(Provisioned()),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster2.Spec.APIEndpoint),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster2.Status.APIEndpoint),
UntilSpaceRequestHasNamespaceAccess(subSpace))
require.NoError(t, err)
VerifyNamespaceAccessForSpaceRequest(t, memberAwait2.Client, spaceRequest) // space request has access to ns on member2
Expand Down Expand Up @@ -272,7 +272,7 @@ func TestCreateSpaceRequest(t *testing.T) {
subSpace, _ = VerifyResourcesProvisionedForSpace(t, awaitilities, subSpace.Name, UntilSpaceHasAnyTargetClusterSet())
_, err = memberAwait.WaitForSpaceRequest(t, types.NamespacedName{Namespace: spaceRequest.GetNamespace(), Name: spaceRequest.GetName()},
UntilSpaceRequestHasConditions(Provisioned()),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Spec.APIEndpoint),
UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Status.APIEndpoint),
UntilSpaceRequestHasNamespaceAccess(subSpace),
UntilSpaceRequestHasNamespaceAccessWithoutSecretRef(), // check that namespace access is present but without a SecretRef set
)
Expand Down Expand Up @@ -323,7 +323,7 @@ func TestUpdateSpaceRequest(t *testing.T) {
)
require.NoError(t, err)

//then
// then
// wait for both spaceRequest and subSpace to have same tierName
subSpace, err = hostAwait.WaitForSpace(t, subSpace.Name,
UntilSpaceHasTier("base"),
Expand Down
47 changes: 0 additions & 47 deletions test/e2e/toolchaincluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,7 @@ func verifyToolchainCluster(t *testing.T, await *wait.Awaitility, otherAwait *wa
secretCopy)

toolchainCluster := newToolchainCluster(await.Namespace, name,
apiEndpoint(current.Spec.APIEndpoint),
caBundle(current.Spec.CABundle),
secretRef(secretCopy.Name),
owner(current.Labels["ownerClusterName"]),
namespace(current.Labels["namespace"]),
disableTLS(current.Spec.DisabledTLSValidations),
capacityExhausted, // make sure this cluster cannot be used in other e2e tests
)

Expand Down Expand Up @@ -117,12 +112,7 @@ func verifyToolchainCluster(t *testing.T, await *wait.Awaitility, otherAwait *wa
client.ObjectKey{Name: name, Namespace: current.Namespace}, secretCopy, kubeconfig.Modify(t, kubeconfig.ApiEndpoint("https://1.2.3.4:8443")))

toolchainCluster := newToolchainCluster(await.Namespace, name,
apiEndpoint("https://1.2.3.4:8443"),
caBundle(current.Spec.CABundle),
secretRef(secretCopy.Name),
owner(current.Labels["ownerClusterName"]),
namespace(current.Labels["namespace"]),
disableTLS(current.Spec.DisabledTLSValidations),
capacityExhausted, // make sure this cluster cannot be used in other e2e tests
)

Expand Down Expand Up @@ -167,8 +157,6 @@ func newToolchainCluster(namespace, name string, options ...clusterOption) *tool
SecretRef: toolchainv1alpha1.LocalSecretReference{
Name: "", // default
},
APIEndpoint: "", // default
CABundle: "", // default
},
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Expand All @@ -192,20 +180,6 @@ var capacityExhausted clusterOption = func(c *toolchainv1alpha1.ToolchainCluster
c.Labels["toolchain.dev.openshift.com/capacity-exhausted"] = strconv.FormatBool(true)
}

// Owner sets the 'ownerClusterName' label
func owner(name string) clusterOption {
return func(c *toolchainv1alpha1.ToolchainCluster) {
c.Labels["ownerClusterName"] = name
}
}

// Namespace sets the 'namespace' label
func namespace(name string) clusterOption {
return func(c *toolchainv1alpha1.ToolchainCluster) {
c.Labels["namespace"] = name
}
}

// SecretRef sets the SecretRef in the cluster's Spec
func secretRef(ref string) clusterOption {
return func(c *toolchainv1alpha1.ToolchainCluster) {
Expand All @@ -214,24 +188,3 @@ func secretRef(ref string) clusterOption {
}
}
}

// APIEndpoint sets the APIEndpoint in the cluster's Spec
func apiEndpoint(url string) clusterOption {
return func(c *toolchainv1alpha1.ToolchainCluster) {
c.Spec.APIEndpoint = url
}
}

// CABundle sets the CABundle in the cluster's Spec
func caBundle(bundle string) clusterOption {
return func(c *toolchainv1alpha1.ToolchainCluster) {
c.Spec.CABundle = bundle
}
}

// disableTLS sets the DisabledTLSValidations field
func disableTLS(validations []toolchainv1alpha1.TLSValidation) clusterOption {
return func(c *toolchainv1alpha1.ToolchainCluster) {
c.Spec.DisabledTLSValidations = validations
}
}
2 changes: 1 addition & 1 deletion test/migration/verify/verify_migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func verifyProvisionedSubSpace(t *testing.T, awaitilities wait.Awaitilities) {
Namespace: subSpaceNamespace,
Name: migration.ProvisionedSpaceRequest,
},
wait.UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Spec.APIEndpoint),
wait.UntilSpaceRequestHasStatusTargetClusterURL(memberCluster.Status.APIEndpoint),
wait.UntilSpaceRequestHasNamespaceAccess(subSpace),
wait.UntilSpaceRequestHasConditions(wait.Provisioned()),
)
Expand Down
2 changes: 1 addition & 1 deletion testsupport/toolchain_status_assertions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func VerifyToolchainStatus(t *testing.T, hostAwait *wait.HostAwaitility, memberA
require.True(t, found)
_, err = hostAwait.WaitForToolchainStatus(t, wait.UntilToolchainStatusHasConditions(wait.ToolchainStatusReadyAndUnreadyNotificationNotCreated()...),
wait.UntilAllMembersHaveUsageSet(),
wait.UntilAllMembersHaveAPIEndpoint(memberCluster.Spec.APIEndpoint),
wait.UntilAllMembersHaveAPIEndpoint(memberCluster.Status.APIEndpoint),
wait.UntilProxyURLIsPresent(hostAwait.APIProxyURL))
require.NoError(t, err, "failed while waiting for ToolchainStatus")
}
Expand Down
10 changes: 3 additions & 7 deletions testsupport/wait/awaitility.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,15 @@ func (a *Awaitility) WaitForToolchainClusterWithCondition(t *testing.T, namespac
// and running in the given expected namespace. It also checks if the CR has the ClusterConditionType
func (a *Awaitility) GetToolchainCluster(t *testing.T, namespace string, cdtype toolchainv1alpha1.ConditionType) (toolchainv1alpha1.ToolchainCluster, bool, error) {
clusters := &toolchainv1alpha1.ToolchainClusterList{}
if err := a.Client.List(context.TODO(), clusters, client.InNamespace(a.Namespace), client.MatchingLabels{
"namespace": namespace,
}); err != nil {
if err := a.Client.List(context.TODO(), clusters, client.InNamespace(a.Namespace)); err != nil {
return toolchainv1alpha1.ToolchainCluster{}, false, err
}
if len(clusters.Items) == 0 {
t.Logf("no toolchaincluster resource with expected labels: namespace='%s'", namespace)
}
// assume there is zero or 1 match only
for _, cl := range clusters.Items {
if cd.IsTrue(cl.Status.Conditions, cdtype) {
if cl.Status.OperatorNamespace == namespace && cd.IsTrue(cl.Status.Conditions, cdtype) {
return cl, true, nil
}
}
Comment on lines 174 to 186
Copy link
Collaborator

Choose a reason for hiding this comment

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

btw, the message in the middle is not up-to-date anymore. Could be modified to:

func (a *Awaitility) GetToolchainCluster(t *testing.T, namespace string, cdtype toolchainv1alpha1.ConditionType) (toolchainv1alpha1.ToolchainCluster, bool, error) {
	clusters := &toolchainv1alpha1.ToolchainClusterList{}
	if err := a.Client.List(context.TODO(), clusters, client.InNamespace(a.Namespace)); err != nil {
		return toolchainv1alpha1.ToolchainCluster{}, false, err
	}
	// assume there is zero or 1 match only
	for _, tCluster := range clusters.Items {
		if tCluster.Status.OperatorNamespace == namespace && cd.IsTrue(tCluster.Status.Conditions, cdtype) {
			return tCluster, true, nil
		}
	}
	t.Logf("no ToolchainCluster resource that would be ready and would contain Status.OperatorNamespace: '%s'", namespace)
	return toolchainv1alpha1.ToolchainCluster{}, false, nil

Expand Down Expand Up @@ -559,9 +557,7 @@ func UntilToolchainClusterHasConditionFalseStatusAndReason(expected toolchainv1a
func UntilToolchainClusterHasOperatorNamespace(expectedNs string) ToolchainClusterWaitCriterion {
return ToolchainClusterWaitCriterion{
Match: func(toolchainCluster *toolchainv1alpha1.ToolchainCluster) bool {
// TODO: remove the check for the legacy label once both host and member operators are upgraded to the
// new version of the operator.
return toolchainCluster.Status.OperatorNamespace == expectedNs || toolchainCluster.Labels["namespace"] == expectedNs
return toolchainCluster.Status.OperatorNamespace == expectedNs
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion testsupport/wait/host.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1416,7 +1416,7 @@ func UntilAllMembersHaveUsageSet() ToolchainStatusWaitCriterion {
func UntilAllMembersHaveAPIEndpoint(apiEndpoint string) ToolchainStatusWaitCriterion {
return ToolchainStatusWaitCriterion{
Match: func(actual *toolchainv1alpha1.ToolchainStatus) bool {
//Since all member operators currently run in the same cluster in the e2e test environment, then using the same memberCluster.Spec.APIEndpoint for all the member clusters should be fine.
// Since all member operators currently run in the same cluster in the e2e test environment, then using the same api endpoint for all the member clusters should be fine.
for _, member := range actual.Status.Members {
// check Member field ApiEndpoint is assigned
if member.APIEndpoint != apiEndpoint {
Expand Down
Loading