Skip to content

Conversation

@kmahar
Copy link
Contributor

@kmahar kmahar commented Aug 31, 2021

Adds testing against load balancers to our Evergreen config.

patch

Also, question: should I add the Ubuntu 18.04 + no auth + no ssl task to our default set of tasks for PRs?

- func: "fix absolute paths"
- func: "bootstrap mongo-orchestration"
vars:
MONGODB_VERSION: "latest"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

technically, we are supposed to test against "each server version that supports load balanced clusters".

this is kind of weird now because no servers actually support it, yet, with the serviceId mocking support they all appear to.

for now, I opted to just test against latest since that's the closest server version to actual support, but I suppose once 5.1 comes out with serviceId support (and we get a "test drivers against 5.1" ticket) we can start testing against both 5.1 and latest.

tasks:
- ".atlas-connect"

- matrix_name: "load-balancer-all"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered doing what we have done for other matrices and having a separate matrix for testing Swift 5.1 (this is necessary since Ubuntu 20.04 doesn't support 5.1). however, I think this is sufficient coverage, and I also relatedly just filed SWIFT-1337 about dropping Swift 5.1 support anyway, per our recent convo / SwiftNIO's decision to do so

Copy link
Contributor

Choose a reason for hiding this comment

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

I think in general we could be a bit looser about what we test, too. The SSWG graduation criteria require that we test at least the two latest versions, and I think if we also include a test that we compile on our minimum supported version, we can be generally confident that the entire range of minimum -> maximum works as expected. In our case that would be compile on 5.1 and test on 5.3 and 5.4. This could help reduce the multiplicative factor of our evergreen config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that's a good point and I agree that just making sure everything builds on those versions seems sufficient, especially given we don't have any branching logic based on language version as far as I can think of.

I filed SWIFT-1340 about reducing the matrix size, seems like a good GBF candidate

- matrix_name: "load-balancer-all"
matrix_spec:
os-fully-featured:
- "ubuntu-18.04"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

haproxy isn't installed by default on macOS like it is on these hosts, and other drivers seem to have skipped running these tests on macOS, so I think just testing on Ubuntu should be sufficient.


public static var multipleMongosLoadBalancedURI: String? {
ProcessInfo.processInfo.environment["MULTIPLE_MONGOS_LB_URI"]
ProcessInfo.processInfo.environment["MULTI_MONGOS_LB_URI"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is actually what the evergreen setup tools name the env variable, I had it wrong

)
]

public static let changeStreamOnCollectionSupport = TestRequirement(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this and the various usages of it are not exactly related to the Evergreen work, however as I was looking through the logs to make sure tests ran correctly I noticed we were incorrectly skipping a bunch of tests which we had hardcoded the supported topologies for and I forgot to add .loadBalanced to so I updated those as well so they would actually run.

@kmahar kmahar marked this pull request as ready for review August 31, 2021 01:58
@kmahar kmahar requested a review from patrickfreed August 31, 2021 01:58
Copy link
Contributor

@patrickfreed patrickfreed left a comment

Choose a reason for hiding this comment

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

should I add the Ubuntu 18.04 + no auth + no ssl task to our default set of tasks for PRs?

SGTM. We also could probably change the PR task to use Swift 5.4 instead of 5.2 as well.

tasks:
- ".atlas-connect"

- matrix_name: "load-balancer-all"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in general we could be a bit looser about what we test, too. The SSWG graduation criteria require that we test at least the two latest versions, and I think if we also include a test that we compile on our minimum supported version, we can be generally confident that the entire range of minimum -> maximum works as expected. In our case that would be compile on 5.1 and test on 5.3 and 5.4. This could help reduce the multiplicative factor of our evergreen config.

@kmahar
Copy link
Contributor Author

kmahar commented Sep 1, 2021

SGTM. We also could probably change the PR task to use Swift 5.4 instead of 5.2 as well.

yeah that's a good idea. will do 👍

@kmahar kmahar merged commit a9e5592 into main Sep 1, 2021
@kmahar kmahar deleted the SWIFT-1328/test-lb-evergreen branch September 1, 2021 17:39
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