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

[JENKINS-62231] Move start logic to postInitialize to avoid OldDataMo… #464

Merged
merged 4 commits into from
May 14, 2020

Conversation

MRamonLeon
Copy link
Contributor

@MRamonLeon MRamonLeon commented May 11, 2020

See JENKINS-62231 for full story

To avoid the plugin to fail when loading its configuration if any modification has been done we move the logic from start to postInitialize method. It may help for future versions upgrade/downgrade ability.

@res0nance @fcojfernandez @alecharp @varyvol @rsandell ARggghhhh I cannot even request reviewers

// backward compatibility with the legacy instance type
Jenkins.XSTREAM.registerConverter(new InstanceTypeConverter());

load();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe just load() is needed

Copy link
Contributor

@res0nance res0nance left a comment

Choose a reason for hiding this comment

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

Looks reasonable

Copy link
Contributor

@fcojfernandez fcojfernandez left a comment

Choose a reason for hiding this comment

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

Seems good, but not tested

@MRamonLeon
Copy link
Contributor Author

MRamonLeon commented May 12, 2020

The build is failing because the recommendedConfiguration uses jenkins.version=2.164.1 and we updated jenkins.version to 2.176.4. We should use a different configuration or revert the bump of the jenkins.version.

@res0nance at this point, taking into account the discussion about recommendedConfigurations in the pipeline-library, the safest way seems to be to specify directly the configuration we need.

def recentLTS = "2.222.3"
def configurations = [
        // Intentionally test configurations which have detected the most problems
        // Linux - Java 8 with plugin specified minimum Jenkins version
        // Windows - Java 8 with recent LTS
        // Linux - Java 11 with recent LTS
        [ platform: "linux", jdk: "8", jenkins: null ],
        // [ platform: "windows", jdk: "8", jenkins: null ],
        // [ platform: "linux", jdk: "8", jenkins: recentLTS, javaLevel: "8" ],
        [ platform: "windows", jdk: "8", jenkins: recentLTS, javaLevel: "8" ],
        [ platform: "linux", jdk: "11", jenkins: recentLTS, javaLevel: "8" ],
        // [ platform: "windows", jdk: "11", jenkins: recentLTS, javaLevel: "8" ]
    ]

I will try in a new commit, not sure if I have the rights to do that.

@MRamonLeon
Copy link
Contributor Author

I believe my changes on Jenkinsfile wasn't taken into account

@res0nance
Copy link
Contributor

There seems to be an issue on windows as well, I'll work on it later, right now I'm trying to get the builds going again quickly

@MRamonLeon
Copy link
Contributor Author

@res0nance would you be able to cut a release with #455 and other improvements? To avoid pain on people upgrading with old ssh clients.

@res0nance res0nance merged commit 77929f2 into jenkinsci:master May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants