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

Explicitly wait for ingress service creation #2445

Merged
merged 2 commits into from
Mar 26, 2024
Merged

Explicitly wait for ingress service creation #2445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0b15e7
connectivity: don't pre-populate the ingress service map
giorio94 Mar 15, 2024
36a66ff
connectivity: explicitly wait for ingress service creation
giorio94 Mar 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 4 additions & 30 deletions connectivity/check/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,29 +875,6 @@ func (ct *ConnectivityTest) deploy(ctx context.Context) error {
if err != nil {
return err
}

ingressServiceName := fmt.Sprintf("cilium-ingress-%s", IngressServiceName)
ct.ingressService[ingressServiceName] = Service{
Service: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: ingressServiceName,
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Name: "http",
Protocol: corev1.ProtocolTCP,
Port: 80,
},
{
Name: "https",
Protocol: corev1.ProtocolTCP,
Port: 443,
},
},
},
},
}
}
}
return nil
Expand Down Expand Up @@ -1295,16 +1272,13 @@ func (ct *ConnectivityTest) validateDeployment(ctx context.Context) error {
}

if ct.Features[features.IngressController].Enabled {
ingressServices, err := ct.clients.src.ListServices(ctx, ct.params.TestNamespace, metav1.ListOptions{LabelSelector: "cilium.io/ingress=true"})
svcName := fmt.Sprintf("cilium-ingress-%s", IngressServiceName)
svc, err := WaitForServiceRetrieval(ctx, ct, ct.client, ct.params.TestNamespace, svcName)
if err != nil {
return fmt.Errorf("unable to list ingress services: %w", err)
return err
}

for _, ingressService := range ingressServices.Items {
ct.ingressService[ingressService.Name] = Service{
Service: ingressService.DeepCopy(),
}
}
ct.ingressService[svcName] = svc
}

if ct.params.MultiCluster == "" {
Expand Down
24 changes: 24 additions & 0 deletions connectivity/check/wait.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/cilium/cilium/api/v1/models"
"golang.org/x/exp/slices"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/cilium/cilium-cli/defaults"
"github.com/cilium/cilium-cli/k8s"
Expand Down Expand Up @@ -134,6 +135,29 @@ func WaitForCoreDNS(ctx context.Context, log Logger, client Pod) error {
}
}

// Service waits until the specified service is created and can be retrieved.
func WaitForServiceRetrieval(ctx context.Context, log Logger, client *k8s.Client, namespace string, name string) (Service, error) {
log.Logf("⌛ [%s] Retrieving service %s/%s ...", client.ClusterName(), namespace, name)

ctx, cancel := context.WithTimeout(ctx, ShortTimeout)
defer cancel()
for {
svc, err := client.GetService(ctx, namespace, name, metav1.GetOptions{})
if err == nil {
return Service{Service: svc.DeepCopy()}, nil
}

log.Debugf("[%s] Failed to retrieve Service %s/%s: %s", client.ClusterName(), namespace, name, err)

select {
case <-time.After(PollInterval):
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better to use inctimer here

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I'd personally keep time.After for consistency with all other usages (inctimer doesn't seem to be used at all in Cilium CLI), and a global change would be out of scope here. It shouldn't matter much in terms of performance, as the CLI is not a long-running application as the Cilium agent, and it is far from being optimized in terms of memory consumption.

case <-ctx.Done():
return Service{}, fmt.Errorf("timeout reached waiting for service %s/%s to be retrieved (last error: %w)",
namespace, name, err)
}
}
}

// WaitForService waits until the given service is synchronized in CoreDNS.
func WaitForService(ctx context.Context, log Logger, client Pod, service Service) error {
log.Logf("⌛ [%s] Waiting for Service %s to become ready...", client.K8sClient.ClusterName(), service.Name())
Expand Down