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

Update Rancher API integration to go-rancher client v2. #2291

Merged
merged 2 commits into from
Nov 5, 2017
Merged

Update Rancher API integration to go-rancher client v2. #2291

merged 2 commits into from
Nov 5, 2017

Conversation

rawmind0
Copy link
Contributor

@rawmind0 rawmind0 commented Oct 20, 2017

Description

  • Update Rancher API integration to go-rancher client v2, to use stacks not environments.
  • Allow the use of labels to autogenerate endpoint, accesskey and secretkey with labels
io.rancher.container.agent.role: environment
io.rancher.container.create_agent: true
  • Set default service name and host rule to <service_name>.<stack_name>.<domain> on rancher integration (api and metadata), to match default dns Rancher naming.

@ldez
Copy link
Contributor

ldez commented Oct 20, 2017

@ldez ldez added the kind/enhancement a new or improved feature. label Oct 20, 2017
@ldez
Copy link
Contributor

ldez commented Oct 20, 2017

@rawmind0 I will manage the glide part 😄

@rawmind0
Copy link
Contributor Author

Thanks @ldez ...Tell me if you need anything else... :)

@ldez
Copy link
Contributor

ldez commented Oct 20, 2017

@rawmind0 done!

@ldez ldez changed the title Update rancher api integration to go-rancher client v2. Update Rancher API integration to go-rancher client v2. Oct 20, 2017
@rawmind0
Copy link
Contributor Author

Thanks so much @ldez .. :)

@ldez ldez added this to the 1.5 milestone Oct 25, 2017
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.

I do not know enough about Rancher to know if the <service_name>. <Stack_name>. <Domain> change is breaking, could you tell me?

} else {
for key, value := range service.LaunchConfig.Labels {
rancherData.Labels[key] = value.(string)
}
}

for _, container := range containers {
if container.Labels[labelRancherStackServiceName] == rancherData.Name &&
if container.Labels[labelRancherStackServiceName] == stack.Name+"/"+service.Name &&
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, the order is different from <service_name>. <stack_name>, could you explain why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because one thing is how system establish context hierarchy names for services stack/service set by container label io.rancher.stack_service.name

Another thing is the dns name for the servicesservice.stack.domain to set default traefik Host matcher.. All rancher dns external integrations and internal dns set by default dns entries as service.stack.domain . I think it's useful to make them match by default.

@rawmind0
Copy link
Contributor Author

rawmind0 commented Oct 31, 2017

This change is just generating default traefik frontend rule to Host: service.stack.domain instead of Host: stack.service.domain , to make it match with default rancher dns entries. You could test that default traefik frontend rule is generated like that.

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 👏

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 👏 🐮

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

@ldez
Copy link
Contributor

ldez commented Nov 3, 2017

ping @SantoDE

Copy link
Collaborator

@SantoDE SantoDE left a comment

Choose a reason for hiding this comment

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

LGTM 👼

I really like that it's a way closer now to match with rancher defaults.

rawmind0 and others added 2 commits November 5, 2017 11:06
…ervice name and host rule to <service_name>.<stack_name>.<domain> on rancher integration
@xaka
Copy link

xaka commented Dec 3, 2017

Isn't backward incompatible change? Going from stack.service.domain to service.stack.domain. Curious whether it should be behind a feature flag otherwise going from 1.4 to 1.5 will be painful. Another option is to mention it as a breaking change in changelog. I'd also love to see it as a backport to 1.4 branch to be better prepared. Great stuff!

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.

7 participants