Skip to content
This repository has been archived by the owner on Aug 4, 2022. It is now read-only.

add support for specifying the name of the endpoint #47

Merged
merged 5 commits into from
Feb 8, 2019

Conversation

lurock
Copy link
Contributor

@lurock lurock commented Jan 28, 2019

You add a "traefik.portName" label to your Service Fabric manifest and specify the name of the endpoint that you want to serve web traffic through.

Example:

<?xml version="1.0" encoding="utf-8"?>
<ServiceManifest Name="WebPkg" Version="1.0.0" xmlns="http://schemas.microsoft.com/2011/01/fabric" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <ServiceTypes>
    <!-- This is the name of your ServiceType.
         The UseImplicitHost attribute indicates this is a guest service. -->
    <StatelessServiceType ServiceTypeName="WebType" UseImplicitHost="true">
      <Extensions>
        <Extension Name="Traefik">
          <Labels xmlns="http://schemas.microsoft.com/2015/03/fabact-no-schema">
            <Label Key="traefik.servicefabric.endpointname">DefaultEndpoint</Label>
            <Label Key="traefik.backend.healthcheck.path">/</Label>
            <Label Key="traefik.backend.healthcheck.port">5050</Label>
            <Label Key="traefik.backend.healthcheck.interval">5s</Label>
            <Label Key="traefik.frontend.priority">10</Label>
            <Label Key="traefik.enable">true</Label>
          </Labels>
        </Extension>
      </Extensions>
    </StatelessServiceType>
  </ServiceTypes>

  <!-- Code package is your service executable. -->
  <CodePackage Name="Code" Version="1.0.0">
      <EntryPoint>
      <ExeHost>
        <Program>Web.exe</Program>
        <WorkingFolder>CodeBase</WorkingFolder>
        <!-- Uncomment to log console output (both stdout and stderr) to one of the
             service's working directories. -->
        <ConsoleRedirection FileRetentionCount="5" FileMaxSizeInKb="2048"/>
      </ExeHost>
    </EntryPoint>
  </CodePackage>
  <ConfigPackage Name="Config" Version="1.0.0" />

  <Resources>
    <Endpoints>
      <!-- This endpoint is used by the communication listener to obtain the port on which to
           listen. Please note that if your service is partitioned, this port is shared with
           replicas of different partitions that are placed in your code. -->
      <Endpoint Name="DefaultEndpoint" UriScheme="http" Port="8000" Protocol="http" Type="Input" />
      <Endpoint Name="HealthCheckEndpoint" UriScheme="http" Port="5050" Protocol="http" Type="Input" />
    </Endpoints>
  </Resources>
</ServiceManifest>

@jjcollinge
Copy link

@lurock thanks, it looks good at first glance :). Would it be possible to get some test coverage for this too please?

@lawrencegripper
Copy link
Contributor

lawrencegripper commented Jan 29, 2019

@lurock Thanks for the PR! This makes sense to me, one tweak I'd propose is maybe moving the label to traefik.servicefabric.endpointname.

My thinking is that existing SF specific labels sit under .servicefabric to help users understand they'll be under the SF docs not the generic Traefik docs and as the name field we're referencing is on the endpoint object in SF I think it would fits better.

Another tweak we could look at would be elevating some of the logic out of the template into a function in go, this isn't a deal breaker for me but @ldez may request it.

@lawrencegripper
Copy link
Contributor

If you want any pointers on this drop onto the Traefik slack and there is a service-fabric channel, happy to chat/help over IM if that is easier.

@lurock
Copy link
Contributor Author

lurock commented Feb 6, 2019

I added a test and made the name changes you requested. Is there more changes needed on this PR?

Copy link
Contributor

@lawrencegripper lawrencegripper left a comment

Choose a reason for hiding this comment

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

Thanks - LGTM. Have you had a chance to give it a spin on a cluster?

@lurock
Copy link
Contributor Author

lurock commented Feb 8, 2019

I have been running it in our production cluster for over a week now and it has been working great.

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.

The template can be simplified: look at my comments 😉

servicefabric_tmpl.go Outdated Show resolved Hide resolved
servicefabric_tmpl.go Outdated Show resolved Hide resolved
servicefabric_tmpl.go Outdated Show resolved Hide resolved
@ldez ldez merged commit 81354a2 into containous:master Feb 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants