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

Add missing functions for ECS template #2312

Merged
merged 1 commit into from
Oct 25, 2017

Conversation

oldmantaiter
Copy link
Contributor

  • Some functions were missing, this adds them to the function
    map passed into the template loader
  • Adds testing around generating ECS configuration
  • This regression was added in ECS provider refactoring #2050

When using ECS, on Traefik start these messages appear in the logs:

time="2017-10-24T14:16:51Z" level=error msg="Provider connection error template: :12: function "getProtocol" not defined, retrying in 8.97478573s"

@oldmantaiter
Copy link
Contributor Author

oldmantaiter commented Oct 24, 2017

Looks like Semaphore is having some issues (docker not running, hosts rebooting), gonna push another and hopefully it goes through.

EDIT: Totally not having issues, I was misunderstanding the host reboot, it is really from failing gofmt (sorry for all the CI noise)

@oldmantaiter oldmantaiter force-pushed the fix-ecs-template branch 3 times, most recently from 9b48ee4 to 7d4a7b8 Compare October 24, 2017 16:29
Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution 👍

"hasStickinessLabel": p.hasStickinessLabel,
"getStickinessCookieName": p.getStickinessCookieName,
// generateECSConfig fills the config template with the given instances
func (p *Provider) generateECSConfig(services map[string][]ecsInstance, ecsFuncMap template.FuncMap) (*types.Configuration, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explains ecsFuncMap? It's always nil...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For ease of testing. I wanted to be able to have a failure case, but I see that is useless now. I will change.

Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

LGTM 👏

- Some functions were missing, this adds them to the function
  map passed into the template loader
- Adds testing around generating ECS configuration
- This regression was added in traefik#2050
Copy link
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants