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

feat: added mergo to handle the differences between versions of jx-requirements.yml when upgrading a cluster #5912

Merged

Conversation

dgozalo
Copy link
Contributor

@dgozalo dgozalo commented Oct 23, 2019

Submitter checklist

  • Change is code complete and matches issue description.
  • Change is covered by existing or new tests.

Description

Created a new function to merge a given requirements struct with the calling one's data and save the output in the given path.

We are using mergo to do so. It will work for any struct and any number of nested structs within the requirements file.

A Transformer however was needed to handle the slice of EnvironmentConfig structs as we want to modify any existing ones with the ones provided in the given requirements file and also append to the slice if there's a new one.

Mergo will always take the calling requirements as base and replace any property with the given requirement's data if and only if the property from the given requirements file is non-zero valued.

Also removed the function that obtained the VersionsStreamConfig from the requirements file because it basically checked that the file existed and returned the versions stream ref struct. These checks are already done when loading the requirements file so it wasn't needed.

Special notes for the reviewer(s)

Which issue this PR fixes

fixes #5896

…quirements.yml when upgrading a cluster

chore: remove unused code
Signed-off-by: Daniel Gozalo <dgozalo@cloudbees.com>

feat: removed an unnecessary func and fixed tests

feat: add wraps around errors

feat: add more wraps
@dgozalo dgozalo changed the title feat: added mergo to handle the differences between versions of jx-re… feat: added mergo to handle the differences between versions of jx-requirements.yml when upgrading a cluster Oct 23, 2019
@dgozalo dgozalo requested a review from daveconde October 23, 2019 15:30
@abayer
Copy link
Contributor

abayer commented Oct 23, 2019

/lgtm

It'd be nice to have a test for how it handles new fields in the "upstream" jx-requirements.yaml that didn't exist in the current jx-requirements.yaml, since I know that's one of the things prompting this, but I'm not sure how one would do that effectively, so hey. =)

@jenkins-x-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abayer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jenkins-x-bot jenkins-x-bot merged commit 94f3546 into jenkins-x:master Oct 23, 2019
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.

jx upgrade boot wipes user requirements
3 participants