Skip to content

OCPBUGS-70147: skip HCP router LB when routes use apps domain #7418

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

Open
celebdor wants to merge 1 commit into
base: release-4.20
Choose a base branch
from
Open

OCPBUGS-70147: skip HCP router LB when routes use apps domain #7418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 5 additions & 20 deletions control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,7 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp *
return fmt.Errorf("failed to reconcile metrics config: %w", err)
}

if useHCPRouter(hcp) {
if routerv2.UseHCPRouter(hcp, r.DefaultIngressDomain) {
if err := r.admitHCPManagedRoutes(ctx, hcp, infraStatus.InternalHCPRouterHost, infraStatus.ExternalHCPRouterHost); err != nil {
return fmt.Errorf("failed to admit HCP managed routes: %w", err)
}
Expand Down Expand Up @@ -1102,6 +1102,7 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp *
MetricsSet: r.MetricsSet,
EnableCIDebugOutput: r.EnableCIDebugOutput,
ImageMetadataProvider: r.ImageMetadataProvider,
DefaultIngressDomain: r.DefaultIngressDomain,
}

var errs []error
Expand All @@ -1115,22 +1116,6 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp *
return utilerrors.NewAggregate(errs)
}

// useHCPRouter returns true if a dedicated common router is created for a HCP to handle ingress for the managed endpoints.
// This is true when the API input specifies intent for the following:
// 1 - AWS endpointAccess is private somehow (i.e. publicAndPrivate or private) or is public and configured with external DNS.
// 2 - When 1 is true, we recommend (and automate via CLI) ServicePublishingStrategy to be "Route" for all endpoints but the KAS
// which needs a dedicated Service type LB external to be exposed if no external DNS is supported.
// Otherwise, the Routes use the management cluster Domain and resolve through the default ingress controller.
func useHCPRouter(hostedControlPlane *hyperv1.HostedControlPlane) bool {
if sharedingress.UseSharedIngress() {
return false
}
if hostedControlPlane.Spec.Platform.Type == hyperv1.IBMCloudPlatform {
return false
}
return util.IsPrivateHCP(hostedControlPlane) || util.IsPublicWithDNS(hostedControlPlane)
}

func IsStorageAndCSIManaged(hostedControlPlane *hyperv1.HostedControlPlane) bool {
if hostedControlPlane.Spec.Platform.Type == hyperv1.IBMCloudPlatform || hostedControlPlane.Spec.Platform.Type == hyperv1.PowerVSPlatform {
return false
Expand Down Expand Up @@ -1392,7 +1377,7 @@ func (r *HostedControlPlaneReconciler) reconcileOLMPackageServerService(ctx cont
}

func (r *HostedControlPlaneReconciler) reconcileHCPRouterServices(ctx context.Context, hcp *hyperv1.HostedControlPlane, createOrUpdate upsert.CreateOrUpdateFN) error {
if sharedingress.UseSharedIngress() || hcp.Spec.Platform.Type == hyperv1.IBMCloudPlatform {
if !routerv2.UseHCPRouter(hcp, r.DefaultIngressDomain) {
return nil
}

Expand Down Expand Up @@ -1421,7 +1406,7 @@ func (r *HostedControlPlaneReconciler) reconcileHCPRouterServices(ctx context.Co
}

// When Public access endpoint we need to create a Service type LB external.
if util.IsPublicWithDNS(hcp) {
if util.IsPublicWithExternalDNS(hcp, r.DefaultIngressDomain) {
if _, err := createOrUpdate(ctx, r.Client, pubSvc, func() error {
return ingress.ReconcileRouterService(pubSvc, false, util.IsPrivateHCP(hcp), hcp)
}); err != nil {
Expand Down Expand Up @@ -1532,7 +1517,7 @@ func (r *HostedControlPlaneReconciler) reconcileInternalRouterServiceStatus(ctx
}

func (r *HostedControlPlaneReconciler) reconcileExternalRouterServiceStatus(ctx context.Context, hcp *hyperv1.HostedControlPlane) (host string, needed bool, message string, err error) {
if !util.IsPublicWithDNS(hcp) || sharedingress.UseSharedIngress() || hcp.Spec.Platform.Type == hyperv1.IBMCloudPlatform {
if !util.IsPublicWithExternalDNS(hcp, r.DefaultIngressDomain) || sharedingress.UseSharedIngress() || hcp.Spec.Platform.Type == hyperv1.IBMCloudPlatform {
return
}
return r.reconcileRouterServiceStatus(ctx, manifests.RouterPublicService(hcp.Namespace), events.NewMessageCollector(ctx, r.Client))
Expand Down
123 changes: 0 additions & 123 deletions control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ import (
"sigs.k8s.io/yaml"

"github.com/go-logr/zapr"
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
"go.uber.org/zap/zaptest"
)
Expand Down Expand Up @@ -2177,125 +2176,3 @@ func componentsFakeDependencies(componentName string, namespace string) []client

return fakeComponents
}

func TestUseHCPRouter(t *testing.T) {
testsCases := []struct {
name string
hcp *hyperv1.HostedControlPlane
expectedResult bool
}{
{
name: "Provider is IBM Cloud",
hcp: &hyperv1.HostedControlPlane{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Namespace: "tenant1",
Name: "cluster1",
},
Spec: hyperv1.HostedControlPlaneSpec{
Platform: hyperv1.PlatformSpec{
Type: hyperv1.IBMCloudPlatform,
},
},
},
expectedResult: false,
},
{
name: "Provider is NonePlatform, services are exposed with Routes",
hcp: &hyperv1.HostedControlPlane{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Namespace: "tenant1",
Name: "cluster1",
},
Spec: hyperv1.HostedControlPlaneSpec{
Platform: hyperv1.PlatformSpec{
Type: hyperv1.NonePlatform,
},
Services: []hyperv1.ServicePublishingStrategyMapping{
{
Service: hyperv1.APIServer,
ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{
Type: hyperv1.Route,
Route: &hyperv1.RoutePublishingStrategy{
Hostname: "cluster1.api.tenant1.com",
},
},
},
{
Service: hyperv1.Konnectivity,
ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{
Type: hyperv1.Route,
Route: &hyperv1.RoutePublishingStrategy{
Hostname: "cluster1.tunnel.tenant1.com",
},
},
},
{
Service: hyperv1.Ignition,
ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{
Type: hyperv1.Route,
Route: &hyperv1.RoutePublishingStrategy{
Hostname: "cluster1.ignition.tenant1.com",
},
},
},
{
Service: hyperv1.OAuthServer,
ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{
Type: hyperv1.Route,
Route: &hyperv1.RoutePublishingStrategy{
Hostname: "cluster1.oauth.tenant1.com",
},
},
},
},
},
},
expectedResult: true,
},
{
name: "Provider is AWS, Public and Private",
hcp: &hyperv1.HostedControlPlane{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Namespace: "tenant1",
Name: "cluster1",
},
Spec: hyperv1.HostedControlPlaneSpec{
Platform: hyperv1.PlatformSpec{
Type: hyperv1.AWSPlatform,
AWS: &hyperv1.AWSPlatformSpec{
EndpointAccess: hyperv1.PublicAndPrivate,
},
},
},
},
expectedResult: true,
},
{
name: "Provider is AWS, Private",
hcp: &hyperv1.HostedControlPlane{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Namespace: "tenant1",
Name: "cluster1",
},
Spec: hyperv1.HostedControlPlaneSpec{
Platform: hyperv1.PlatformSpec{
Type: hyperv1.AWSPlatform,
AWS: &hyperv1.AWSPlatformSpec{
EndpointAccess: hyperv1.Private,
},
},
},
},
expectedResult: true,
},
}
for _, tc := range testsCases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expectedResult, useHCPRouter(tc.hcp))
})
}
}
31 changes: 19 additions & 12 deletions control-plane-operator/controllers/hostedcontrolplane/v2/router/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ func (k *router) NeedsManagementKASAccess() bool {

func NewComponent() component.ControlPlaneComponent {
return component.NewDeploymentComponent(ComponentName, &router{}).
WithPredicate(useHCPRouter).
WithPredicate(func(cpContext component.WorkloadContext) (bool, error) {
return UseHCPRouter(cpContext.HCP, cpContext.DefaultIngressDomain), nil
}).
WithManifestAdapter(
"config.yaml",
component.WithAdaptFunction(adaptConfig),
Expand All @@ -47,18 +49,23 @@ func NewComponent() component.ControlPlaneComponent {
Build()
}

// useHCPRouter returns true if a dedicated common router is created for a HCP to handle ingress for the managed endpoints.
// This is true when the API input specifies intent for the following:
// 1 - AWS endpointAccess is private somehow (i.e. publicAndPrivate or private) or is public and configured with external DNS.
// 2 - When 1 is true, we recommend (and automate via CLI) ServicePublishingStrategy to be "Route" for all endpoints but the KAS
// which needs a dedicated Service type LB external to be exposed if no external DNS is supported.
// Otherwise, the Routes use the management cluster Domain and resolve through the default ingress controller.
func useHCPRouter(cpContext component.WorkloadContext) (bool, error) {
// UseHCPRouter returns true if a dedicated HCP router is needed to handle ingress for managed endpoints.
// This is true when:
// 1 - Shared ingress is not enabled, AND
// 2 - Platform is not IBMCloud, AND
// 3 - AWS endpointAccess is private (i.e. publicAndPrivate or private), OR
// 4 - The HCP is public and has services configured with Route hostnames external to the
//
// management cluster's default ingress domain.
//
// When hostnames are subdomains of the apps domain, they are served by the management cluster's
// default router via wildcard DNS, so no dedicated HCP router is needed.
func UseHCPRouter(hcp *hyperv1.HostedControlPlane, defaultIngressDomain string) bool {
if sharedingress.UseSharedIngress() {
return false, nil
return false
}
if cpContext.HCP.Spec.Platform.Type == hyperv1.IBMCloudPlatform {
return false, nil
if hcp.Spec.Platform.Type == hyperv1.IBMCloudPlatform {
return false
}
return util.IsPrivateHCP(cpContext.HCP) || util.IsPublicWithDNS(cpContext.HCP), nil
return util.IsPrivateHCP(hcp) || util.IsPublicWithExternalDNS(hcp, defaultIngressDomain)
}
Loading