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.

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

Dialer is broken in e2e tests #1273

Closed
barkbay opened this issue Jul 17, 2019 · 4 comments · Fixed by #1297
Closed

Dialer is broken in e2e tests #1273

barkbay opened this issue Jul 17, 2019 · 4 comments · Fixed by #1297
Assignees
Labels
>bug Something isn't working

Comments

@barkbay
Copy link
Contributor

barkbay commented Jul 17, 2019

dialer is broken in e2e tests.
I think it comes from e1a3023#diff-fcadcd8abc54d908e7568bc91579f44fL31

The namespace is needed in the url in order to get the pod with the RESTClient and without the svc it's trying to forward to a pod and not to a service.

@thbkrkr
Copy link
Contributor

thbkrkr commented Jul 18, 2019

This happens when using --auto-port-forward.

The custom dev port forwarder expects that a pod address is composed of three parts <name>.<namespace>.svc:

// retrieve pod name and namespace from addr
// TODO: subdomains in pod names would change this.
parts := strings.SplitN(host, ".", 3)
if len(parts) <= 2 {
return nil, fmt.Errorf("unsupported pod address format: %s", host)
}
return &types.NamespacedName{Namespace: parts[1], Name: parts[0]}, nil

@thbkrkr thbkrkr added the >bug Something isn't working label Jul 18, 2019
@sebgl
Copy link
Contributor

sebgl commented Jul 18, 2019

I guess we can either:

  1. Revert the e2e change to keep using name.namespace.svc
  2. Modify the port forward code to be able to forward name only

I'd vote for 2.

@thbkrkr
Copy link
Contributor

thbkrkr commented Jul 18, 2019

  1. Modify the port forward code to be able to forward name only

Do not we also need the namespace?

@sebgl
Copy link
Contributor

sebgl commented Jul 18, 2019

Our quickstart doc advertises reaching the service from its name only (no namespace, no .svc extension):

Get access to Elasticsearch.
From the Kubernetes cluster, use this URL:
https://quickstart-es:9200

However this longer-term E2E tests will probably work across multiple namespaces I guess it's fine to include the namespace here. With no namespace it's almost impossible to know where to redirect the call from the custom dialer code perspective anyway. tl;dr: you're right 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants