Skip to content
This repository was archived by the owner on Dec 12, 2025. It is now read-only.

Conversation

@chatton
Copy link
Contributor

@chatton chatton commented Mar 23, 2020

All Submissions:

  • Have you opened an Issue before filing this PR?
  • Have you signed our CLA?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if such).

This PR adds a local copy of the version manifest to the operator image. When constructing the automation config, only the relevant builds are added.

An e2e test will be added which tests version change after the test improvements made in #28

@chatton chatton requested a review from rodrigovalin March 23, 2020 13:30
Copy link
Contributor

@rodrigovalin rodrigovalin left a comment

Choose a reason for hiding this comment

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

Missing unit test on BuildsForVersion please

# TODO: This build takes longer than it needs to and can still be optimized
RUN go build -o build/_output/bin/mongodb-kubernetes-operator -mod=vendor github.com/mongodb/mongodb-kubernetes-operator/cmd/manager

ENV manifest_version=4.2
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be on the next stage? it is not needed at this point I guess.

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 will likely change, I was thinking for now we can just grab the version for 4.2. This can be adjusted later

func (v VersionManifest) BuildsForVersion(version string) MongoDbVersionConfig {
var builds []BuildConfig
for _, versionConfig := range v.Versions {
if versionConfig.Name != version || strings.HasSuffix(version, "-ent") {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the idea behind the right part of the if ... a version suffixed with -ent will never get any builds, and there are no unit tests checking this ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right I had this mixed up with something else, we don't need this 👍

@chatton
Copy link
Contributor Author

chatton commented Mar 23, 2020

@rodrigovalin tests added for BuildsForVersion

Also added a versionManifest function as argument for mocking purposes. (tests were looking for file on disk)

Copy link
Contributor

@rodrigovalin rodrigovalin left a comment

Choose a reason for hiding this comment

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

🐺

@chatton chatton merged commit 1ae5cee into master Mar 23, 2020
@chatton chatton deleted the CLOUDP-58413_deploy_any_version_mongodb branch March 23, 2020 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants