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

Add matrix for ruby diff values #324

Merged
merged 1 commit into from Mar 28, 2023
Merged

Add matrix for ruby diff values #324

merged 1 commit into from Mar 28, 2023

Conversation

archanaserver
Copy link
Contributor

@archanaserver archanaserver commented Mar 24, 2023

For reference:

#317 (comment)

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Thanks for working on the fix.

You're missing the steps to run within the matrix. I'm not sure anyone linked it to you yet, but I always find https://www.jenkins.io/doc/book/pipeline/syntax/ to be useful. https://www.jenkins.io/doc/book/pipeline/syntax/#matrix-stages in this particular case.

Could you also rebase? We prefer to avoid merge commits in our PRs.

@archanaserver
Copy link
Contributor Author

Thanks for working on the fix.

You're missing the steps to run within the matrix. I'm not sure anyone linked it to you yet, but I always find https://www.jenkins.io/doc/book/pipeline/syntax/ to be useful. https://www.jenkins.io/doc/book/pipeline/syntax/#matrix-stages in this particular case.

@ekohl This was helpful. I fixed the syntax now, but I'm facing issue to look for error. Can you help me with this?

Could you also rebase? We prefer to avoid merge commits in our PRs.

Yes, it's now rebased.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I'll admit I also need to look this up. You can run the tests locally. We have test and CI simply executes that. So a simple invocation just testing our theforeman.org directory:

./test theforeman.org

This will run:

cd theforeman.org
jenkins-jobs -l debug test -r yaml

(and then some more)

You can also pass a job name if you do it manully:

jenkins-jobs -l debug test -r yaml smart-proxy-develop-source-release

Now that part actually passes, but if you're changing the yaml files then it's very useful.

The other part is that it runs jenkins-lint.py.

To run that, it first generates all the XML:

jenkins-jobs -l debug test -r yaml --config-xml -o output

Then in output you have all the XML files. You're interested in output/smart-proxy-develop-source-release/config.xml, so:

$ ./jenkins-lint.py --xml theforeman.org/output/smart-proxy-develop-source-release/config.xml
{'error': ['Undefined section "matrix" @ line 21, column 5.', 'Missing required section "stages" @ line 11, column 1.']}
theforeman.org/output/smart-proxy-develop-source-release/config.xml: NOT OK
Failed to validate files:
{'error': ['Undefined section "matrix" @ line 21, column 5.', 'Missing required section "stages" @ line 11, column 1.']}
theforeman.org/output/smart-proxy-develop-source-release/config.xml: NOT OK

If you make sure that passes, I'd expect CI to pass as well.

run_test(ruby)
}
}
stage('Build and Archive Source') {
Copy link
Member

Choose a reason for hiding this comment

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

Having this in every stage in the matrix means we'll generate 3 sources. I'm unsure if only one gets used, but it's a bit redundant. You can keep this stage outside of the matrix.

stage("Clone repository") {
steps {
git url: git_url, branch: git_ref
matrix {
Copy link
Member

Choose a reason for hiding this comment

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

This is in the error output:

2023-03-24T16:20:03.7780551Z {'error': ['Undefined section "matrix" @ line 21, column 5.', 'Missing required section "stages" @ line 11, column 1.']}

I think you need this kind of structure:

  • stages
    • stage: 'test'
      • matrix
      • stages
        • stage: clone
        • stage: Test Ruby
    • stage: Build and Archive Source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ekohl Thanks for the detailed explaination and sharing those resources, it helped! Also locally it is passing all checks and this is the output i got: theforeman.org/output/smart-proxy-develop-source-release/config.xml: OK

(and now i can see here too, it passing all the checks) :)

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this.

@ekohl ekohl merged commit 68d89e6 into theforeman:master Mar 28, 2023
2 checks passed
}
stage("Test Ruby") {
steps {
run_test(ruby)
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't find this variable and I wonder why. Looking at https://www.jenkins.io/doc/book/pipeline/syntax/#declarative-matrix I think it should be

Suggested change
run_test(ruby)
run_test(env.ruby)

Though the docs aren't very clear on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ekohl i guess i've created some git mess here, can you help?
here is the change: #326
(it is fixed, thanks!)

@archanaserver archanaserver mentioned this pull request Mar 29, 2023
ekohl pushed a commit that referenced this pull request Mar 31, 2023
ekohl pushed a commit that referenced this pull request Mar 31, 2023
This reverts commit faddd33.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants