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

Issue 434 - step1 #440

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

cazacugmihai
Copy link

In this step we have added support for implementing the grouped services.
No functionality has been changed.

@vfarcic
Copy link
Owner

vfarcic commented Feb 27, 2018

I hope I'll have time to review this on Friday. Maybe @thomasjpfan can go through it sooner.

@thomasjpfan
Copy link
Contributor

thomasjpfan commented Feb 28, 2018

An implementation that would change the least amount of code would be to add a GroupServices map[string][]Service to the Data struct:

type Data struct {
    Services map[string]Service
    GroupServices map[string][]Service
}

And handle the GroupServices separately when generating the haproxy configuraiton. We would have to make sure that the keys in Services and GroupServices do not overlap.

@cazacugmihai
Copy link
Author

There is no enough space for running the tests: No space left on device.

@vfarcic
Copy link
Owner

vfarcic commented Mar 4, 2018

There was a problem with the latest haproxy release. It's resolved by specifying a fixed version in Dockerfile. If you merge your code with the master branch, it should work or, at least, not fail for the same reason.

@cazacugmihai
Copy link
Author

I have just merged it with the upstream.

@vfarcic
Copy link
Owner

vfarcic commented Mar 5, 2018

Please take a look at https://jenkins.dockerflow.com/blue/organizations/jenkins/vfarcic%2Fdocker-flow-proxy/detail/PR-440/7/pipeline/ . Some of the integration tests are failing with those changes.

I'll do my best to review the changes as soon as possible. However, given my current agenda, I might not be able to do that before the upcoming weekend. I hope that's OK.

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

Successfully merging this pull request may close these issues.

3 participants