Skip to content

Commit 5a5f3ab

Browse files
alex-bezekalex-bezek
authored
Push events to ingress about domain conditions via driver (#730)
1 parent 5bc61e5 commit 5a5f3ab

File tree

4 files changed

+217
-24
lines changed

4 files changed

+217
-24
lines changed

‎cmd/api-manager.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options apiM
486486
managerdriver.WithClusterDomain(options.clusterDomain),
487487
managerdriver.WithDisableGatewayReferenceGrants(options.disableGatewayReferenceGrants),
488488
managerdriver.WithDefaultDomainReclaimPolicy(defaultDomainReclaimPolicy),
489+
managerdriver.WithEventRecorder(mgr.GetEventRecorderFor("k8s-resource-driver")),
489490
}
490491

491492
if tcpRouteCRDInstalled {

‎internal/testutils/k8s-resources.go‎

Lines changed: 63 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,39 +48,46 @@ func NewTestIngressV1WithClass(name string, namespace string, ingressClass strin
4848
return i
4949
}
5050

51-
func NewTestIngressV1(name string, namespace string) *netv1.Ingress {
52-
return &netv1.Ingress{
53-
ObjectMeta: metav1.ObjectMeta{
54-
Name: name,
55-
Namespace: namespace,
56-
},
57-
Spec: netv1.IngressSpec{
58-
Rules: []netv1.IngressRule{
59-
{
60-
Host: "example.com",
61-
IngressRuleValue: netv1.IngressRuleValue{
62-
HTTP: &netv1.HTTPIngressRuleValue{
63-
Paths: []netv1.HTTPIngressPath{
64-
{
65-
Path: "/",
66-
Backend: netv1.IngressBackend{
67-
Service: &netv1.IngressServiceBackend{
68-
Name: "example",
69-
Port: netv1.ServiceBackendPort{
70-
Number: 80,
71-
},
72-
},
51+
// NewTestIngressV1WithHosts creates an ingress with the specified hosts.
52+
func NewTestIngressV1WithHosts(name string, namespace string, hosts ...string) *netv1.Ingress {
53+
rules := make([]netv1.IngressRule, 0, len(hosts))
54+
for _, host := range hosts {
55+
rules = append(rules, netv1.IngressRule{
56+
Host: host,
57+
IngressRuleValue: netv1.IngressRuleValue{
58+
HTTP: &netv1.HTTPIngressRuleValue{
59+
Paths: []netv1.HTTPIngressPath{
60+
{
61+
Path: "/",
62+
Backend: netv1.IngressBackend{
63+
Service: &netv1.IngressServiceBackend{
64+
Name: "example",
65+
Port: netv1.ServiceBackendPort{
66+
Number: 80,
7367
},
7468
},
7569
},
7670
},
7771
},
7872
},
7973
},
74+
})
75+
}
76+
return &netv1.Ingress{
77+
ObjectMeta: metav1.ObjectMeta{
78+
Name: name,
79+
Namespace: namespace,
80+
},
81+
Spec: netv1.IngressSpec{
82+
Rules: rules,
8083
},
8184
}
8285
}
8386

87+
func NewTestIngressV1(name string, namespace string) *netv1.Ingress {
88+
return NewTestIngressV1WithHosts(name, namespace, "example.com")
89+
}
90+
8491
func NewTestServiceV1(name string, namespace string) *corev1.Service {
8592
return &corev1.Service{
8693
ObjectMeta: metav1.ObjectMeta{
@@ -100,8 +107,26 @@ func NewTestServiceV1(name string, namespace string) *corev1.Service {
100107
}
101108
}
102109

103-
func NewDomainV1(name string, namespace string) *ingressv1alpha1.Domain {
104-
return &ingressv1alpha1.Domain{
110+
type DomainOpt func(*ingressv1alpha1.Domain)
111+
112+
// WithDomainStatusCondition adds a condition to the domain's status.
113+
func WithDomainStatusCondition(condition metav1.Condition) DomainOpt {
114+
return func(d *ingressv1alpha1.Domain) {
115+
d.Status.Conditions = append(d.Status.Conditions, condition)
116+
}
117+
}
118+
119+
// WithDomainReadyCondition is a convenience helper that sets the Ready condition.
120+
func WithDomainReadyCondition(status metav1.ConditionStatus, message string) DomainOpt {
121+
return WithDomainStatusCondition(metav1.Condition{
122+
Type: "Ready",
123+
Status: status,
124+
Message: message,
125+
})
126+
}
127+
128+
func NewDomainV1(name string, namespace string, opts ...DomainOpt) *ingressv1alpha1.Domain {
129+
d := &ingressv1alpha1.Domain{
105130
ObjectMeta: metav1.ObjectMeta{
106131
Name: ingressv1alpha1.HyphenatedDomainNameFromURL(name),
107132
Namespace: namespace,
@@ -110,6 +135,10 @@ func NewDomainV1(name string, namespace string) *ingressv1alpha1.Domain {
110135
Domain: name,
111136
},
112137
}
138+
for _, opt := range opts {
139+
opt(d)
140+
}
141+
return d
113142
}
114143

115144
// RandomName generates a random name with the given prefix. The random part is 5 characters long.
@@ -319,3 +348,13 @@ func FindCondition(conditions []metav1.Condition, condType string) *metav1.Condi
319348
}
320349
return nil
321350
}
351+
352+
// NewDomainMap creates a map of domain name to Domain objects, useful for testing
353+
// functions that operate on domain lookups.
354+
func NewDomainMap(domains ...*ingressv1alpha1.Domain) map[string]ingressv1alpha1.Domain {
355+
result := make(map[string]ingressv1alpha1.Domain, len(domains))
356+
for _, d := range domains {
357+
result[d.Spec.Domain] = *d
358+
}
359+
return result
360+
}

‎pkg/managerdriver/driver.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1818
"k8s.io/apimachinery/pkg/runtime"
1919
"k8s.io/apimachinery/pkg/types"
20+
"k8s.io/client-go/tools/record"
2021
"k8s.io/client-go/util/retry"
2122
"k8s.io/utils/ptr"
2223
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -67,6 +68,8 @@ type Driver struct {
6768
disableGatewayReferenceGrants bool
6869

6970
defaultDomainReclaimPolicy *ingressv1alpha1.DomainReclaimPolicy
71+
72+
recorder record.EventRecorder
7073
}
7174

7275
type DriverOpt func(*Driver)
@@ -113,6 +116,12 @@ func WithDefaultDomainReclaimPolicy(policy ingressv1alpha1.DomainReclaimPolicy)
113116
}
114117
}
115118

119+
func WithEventRecorder(recorder record.EventRecorder) DriverOpt {
120+
return func(d *Driver) {
121+
d.recorder = recorder
122+
}
123+
}
124+
116125
// NewDriver creates a new driver with a basic logger and cache store setup
117126
func NewDriver(logger logr.Logger, scheme *runtime.Scheme, controllerName string, managerName types.NamespacedName, opts ...DriverOpt) *Driver {
118127
cacheStores := store.NewCacheStores(logger)
@@ -648,6 +657,8 @@ func (d *Driver) updateStatuses(ctx context.Context, c client.Client) error {
648657
// updateIngressesStatuses iterates over all ingresses and updates their statuses if
649658
// they need it. It does this by calculating the domains for each ingress and checking
650659
// against the DomainCRs CNAMETargets to propery set the LoadBalancer.Ingress status.
660+
// It also records events to ingresses based on domain Ready conditions to help users
661+
// understand domain-related issues without needing to inspect the Domain CRs directly.
651662
func (d *Driver) updateIngressStatuses(ctx context.Context, c client.Client) error {
652663
domains, err := getDomainsByDomain(ctx, c)
653664
if err != nil {
@@ -662,6 +673,9 @@ func (d *Driver) updateIngressStatuses(ctx context.Context, c client.Client) err
662673
continue
663674
}
664675

676+
// Record events from domain Ready conditions to help users understand domain issues
677+
d.recordDomainEventsForIngress(ingress, domains)
678+
665679
newLBIPStatus := calculateIngressLoadBalancerIPStatus(ingress, domains)
666680
if reflect.DeepEqual(ingress.Status.LoadBalancer.Ingress, newLBIPStatus) {
667681
continue
@@ -691,6 +705,38 @@ func (d *Driver) updateIngressStatuses(ctx context.Context, c client.Client) err
691705
return g.Wait()
692706
}
693707

708+
// recordDomainEventsForIngress records events to the ingress based on the Ready condition
709+
// of its associated domains. This helps users understand domain-related issues (e.g., using
710+
// an invalid domain on a free account) without needing to inspect Domain CRs directly.
711+
func (d *Driver) recordDomainEventsForIngress(ingress *netv1.Ingress, domains map[string]ingressv1alpha1.Domain) {
712+
if d.recorder == nil {
713+
return
714+
}
715+
716+
for _, rule := range ingress.Spec.Rules {
717+
domain, ok := domains[rule.Host]
718+
if !ok {
719+
continue
720+
}
721+
722+
readyCondition := meta.FindStatusCondition(domain.Status.Conditions, "Ready")
723+
if readyCondition == nil {
724+
continue
725+
}
726+
727+
if readyCondition.Status == metav1.ConditionFalse {
728+
d.recorder.Eventf(
729+
ingress,
730+
corev1.EventTypeWarning,
731+
"DomainNotReady",
732+
"Domain %q is not ready: %s",
733+
rule.Host,
734+
readyCondition.Message,
735+
)
736+
}
737+
}
738+
}
739+
694740
func (d *Driver) updateGatewayStatuses(ctx context.Context, c client.Client) error {
695741
domains, err := getDomainsByDomain(ctx, c)
696742
if err != nil {

‎pkg/managerdriver/driver_test.go‎

Lines changed: 107 additions & 0 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/apimachinery/pkg/runtime"
1515
"k8s.io/apimachinery/pkg/types"
16+
"k8s.io/client-go/tools/record"
1617
"k8s.io/utils/ptr"
1718
"sigs.k8s.io/controller-runtime/pkg/client"
1819
"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -1249,3 +1250,109 @@ func TestExtractPolicy(t *testing.T) {
12491250
})
12501251
}
12511252
}
1253+
1254+
var _ = Describe("RecordDomainEventsForIngress", func() {
1255+
var driver *Driver
1256+
var fakeRecorder *record.FakeRecorder
1257+
var scheme = runtime.NewScheme()
1258+
1259+
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
1260+
utilruntime.Must(ingressv1alpha1.AddToScheme(scheme))
1261+
1262+
BeforeEach(func() {
1263+
fakeRecorder = record.NewFakeRecorder(10)
1264+
driver = NewDriver(
1265+
GinkgoLogr,
1266+
scheme,
1267+
testutils.DefaultControllerName,
1268+
types.NamespacedName{Name: defaultManagerName},
1269+
WithEventRecorder(fakeRecorder),
1270+
)
1271+
})
1272+
1273+
It("Should record warning event when domain Ready condition is false", func() {
1274+
ingress := testutils.NewTestIngressV1WithHosts("test-ingress", "default", "example.com")
1275+
domains := testutils.NewDomainMap(
1276+
testutils.NewDomainV1("example.com", "default",
1277+
testutils.WithDomainReadyCondition(metav1.ConditionFalse, "domain not available on free plan")),
1278+
)
1279+
1280+
driver.recordDomainEventsForIngress(ingress, domains)
1281+
1282+
Expect(fakeRecorder.Events).To(Receive(Equal(
1283+
"Warning DomainNotReady Domain \"example.com\" is not ready: domain not available on free plan",
1284+
)))
1285+
})
1286+
1287+
It("Should not record event when domain Ready condition is true", func() {
1288+
ingress := testutils.NewTestIngressV1WithHosts("test-ingress", "default", "valid.ngrok.io")
1289+
domains := testutils.NewDomainMap(
1290+
testutils.NewDomainV1("valid.ngrok.io", "default",
1291+
testutils.WithDomainReadyCondition(metav1.ConditionTrue, "")),
1292+
)
1293+
1294+
driver.recordDomainEventsForIngress(ingress, domains)
1295+
1296+
Expect(fakeRecorder.Events).ToNot(Receive())
1297+
})
1298+
1299+
It("Should not record event when domain not found", func() {
1300+
ingress := testutils.NewTestIngressV1WithHosts("test-ingress", "default", "unknown.com")
1301+
domains := testutils.NewDomainMap()
1302+
1303+
driver.recordDomainEventsForIngress(ingress, domains)
1304+
1305+
Expect(fakeRecorder.Events).ToNot(Receive())
1306+
})
1307+
1308+
It("Should not record event when domain has no Ready condition", func() {
1309+
ingress := testutils.NewTestIngressV1WithHosts("test-ingress", "default", "pending.com")
1310+
domains := testutils.NewDomainMap(
1311+
testutils.NewDomainV1("pending.com", "default"),
1312+
)
1313+
1314+
driver.recordDomainEventsForIngress(ingress, domains)
1315+
1316+
Expect(fakeRecorder.Events).ToNot(Receive())
1317+
})
1318+
1319+
It("Should record events for multiple failing domains", func() {
1320+
ingress := testutils.NewTestIngressV1WithHosts("test-ingress", "default", "fail1.com", "fail2.com")
1321+
domains := testutils.NewDomainMap(
1322+
testutils.NewDomainV1("fail1.com", "default",
1323+
testutils.WithDomainReadyCondition(metav1.ConditionFalse, "error 1")),
1324+
testutils.NewDomainV1("fail2.com", "default",
1325+
testutils.WithDomainReadyCondition(metav1.ConditionFalse, "error 2")),
1326+
)
1327+
1328+
driver.recordDomainEventsForIngress(ingress, domains)
1329+
1330+
var events []string
1331+
for i := 0; i < 2; i++ {
1332+
var event string
1333+
Expect(fakeRecorder.Events).To(Receive(&event))
1334+
events = append(events, event)
1335+
}
1336+
Expect(events).To(ContainElement("Warning DomainNotReady Domain \"fail1.com\" is not ready: error 1"))
1337+
Expect(events).To(ContainElement("Warning DomainNotReady Domain \"fail2.com\" is not ready: error 2"))
1338+
})
1339+
1340+
It("Should not panic when recorder is nil", func() {
1341+
driverNoRecorder := NewDriver(
1342+
GinkgoLogr,
1343+
scheme,
1344+
testutils.DefaultControllerName,
1345+
types.NamespacedName{Name: defaultManagerName},
1346+
)
1347+
1348+
ingress := testutils.NewTestIngressV1WithHosts("test-ingress", "default", "example.com")
1349+
domains := testutils.NewDomainMap(
1350+
testutils.NewDomainV1("example.com", "default",
1351+
testutils.WithDomainReadyCondition(metav1.ConditionFalse, "some error")),
1352+
)
1353+
1354+
Expect(func() {
1355+
driverNoRecorder.recordDomainEventsForIngress(ingress, domains)
1356+
}).ToNot(Panic())
1357+
})
1358+
})

0 commit comments

Comments
 (0)