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

Segments Labels: Mesos #3383

Merged
merged 8 commits into from
Jun 5, 2018
Merged

Conversation

drewkerrigan
Copy link
Contributor

@drewkerrigan drewkerrigan commented May 24, 2018

What does this PR do?

  • Add support for segment labels for Mesos
    • Also added behavior such that if the segment name matches a named port, and no other port selector labels are used, then that port will be used for that segment
  • Add new traefik.portName label
    • Only used by Mesos currently, but could potentially be used by Marathon as well
    • Used as port selector for mesos tasks with multiple named ports
  • Added tests in provider/mesos/config_test.go for both of the above
  • Added segment label documentation to docs/configuration/backends/mesos.md
  • Added some additional template functions (not required by existing templates, so backwards compatible)

Motivation

Segment labels were missing from the mesos provider and they are obviously very useful and sometimes required for mesos frameworks running many multi-port tasks.

Related to #3383

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

Regarding the design: My primary goal was to keep these changes backwards compatible with the existing templates, so I kept the taskData object basically the same except for the addition of the SegmentName. This approach means that when there are segments, the state.Task is duplicated per segment, but it also means that the majority of the code and templates using taskData.TraefikLabels and the embedded state.Task can remain the same.

@ldez ldez added this to the 1.7 milestone May 24, 2018
@ldez ldez self-requested a review May 24, 2018 23:40
@ldez ldez self-assigned this May 24, 2018
@ldez ldez mentioned this pull request May 24, 2018
8 tasks
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
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

@ldez ldez requested a review from juliens June 5, 2018 06:43
Copy link
Member

@juliens juliens 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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants