search

openshift / puppet-openshift_origin

Puppet module to create OpenShift Brokers and Nodes. Can be used to create a full OpenShift Origin deployment.
http://forge.puppetlabs.com/openshift/openshift_origin
Other
45 stars 128 forks source link

Adding Nginx routing SPI for HA applications #312

Closed andrewklau closed 10 months ago

andrewklau commented 10 years ago

[test]

sdodson commented 10 years ago

[test]

openshift-bot commented 10 years ago

Origin Test Results: FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests/3231/)

sdodson commented 10 years ago

I'll give this a quick validation today and merge after that looks good.

andrewklau commented 10 years ago

Sorry, I hadn't had any time to test this. Things have been hectic here, thanks for the changes.

sdodson commented 10 years ago

No worries, today I got end to end testing complete in my environment. I sent you more changes for your PR. I haven't had a chance to test any of this in a less distributed environment but this works for me in an environment where broker, msgserver, and nginx roles are all on different hosts.

Unless you have any outstanding concerns I think we're good to go.

andrewklau commented 10 years ago

Sounds good. If you're happy with it, I have no objections.

In a few weeks, I plan on investigating further how to extend it to include multiple nginx roles and region support. Sadly time has been a factor hence the partial pull request.

Thanks.

andrewklau commented 10 years ago

Also, I can squash and rebase it if you're happy with it.

sdodson commented 10 years ago

[test]

sdodson commented 10 years ago

[test]

sdodson commented 10 years ago

@andrewklau We did some pretty significant refactoring to manage dependencies between classes better. Do you mind rebasing on top of master? Sorry for the troubles.

andrewklau commented 10 years ago

@sdodson done

andrewklau commented 10 years ago

[test]

sdodson commented 10 years ago

[test] just to smoke test then we'll merge

andrewklau commented 10 years ago

oops, I rushed this and didn't run any validation. Fixed now. Sorry

andrewklau commented 9 years ago

Not sure why it errors at: manifests/broker.pp:286:trailing_whitespace:ERROR:trailing whitespace found

sdodson commented 9 years ago

@andrewklau Hmm, the line numbers don't match what Travis is saying, but I see whitespace. I'll annotate.

andrewklau commented 9 years ago

@sdodson thanks, looks like it's passing now.

sdodson commented 9 years ago

[test]

andrewklau commented 9 years ago

Is there a way I can see why it failed?

sdodson commented 9 years ago

[test] test results were purged

openshift-bot commented 9 years ago

Evaluated for origin up to acc3d3c128f02c46d0f03bd9f3f3f44ac092f89b

sdodson commented 9 years ago

@andrewklau Can you view this? I don't think you have to be on Red Hat's VPN for it but I'm not 100% certain. If so search for ') Error:' to find the test failures.

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests/3231/consoleFull

The tests are failing because they expect ALLOW_HA_APPLICATIONS="true". We could only adjust DEFAULT_ALLOW_HA, leaving ALLOW_HA_APPLICATIONS="true" regardless of whether routing is enabled or not. While that seems like a hack to work around these tests, it is how the default config is deployed. However I don't think there's much point in ALLOW_HA_APPLICATIONS='true' without an HA routing layer. So even ignoring the automated test suite for a minute, I'm not sure what the best course of action here is.

Specific to the test suite, I can modify it to enable routing.

andrewklau commented 9 years ago

@sdodson Thanks, yes I can. Let me know if I need to modify anything else here, perhaps I could modify it for the test suite and return it back if it's successful.

openshift-bot commented 9 years ago

Origin Action Required: Please contact #openshift-dev to have this pull request manually reviewed and tested