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

Implement agent installation #4

Merged
merged 9 commits into from
Jan 21, 2021
Merged

Implement agent installation #4

merged 9 commits into from
Jan 21, 2021

Conversation

j-rivero
Copy link
Contributor

The PR implements the installation of the Jenkins agent in the node. Mostly adapted from https://github.com/ros-infrastructure/cookbook-ros-buildfarm

Copy link
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

Some questions and non-blocking comments.

attributes/default.rb Outdated Show resolved Hide resolved
recipes/default.rb Show resolved Hide resolved
service 'jenkins-agent' do
action [:start, :enable]
# can not connect to server while testing
not_if { node.chef_environment == "test" }
Copy link
Member

Choose a reason for hiding this comment

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

Earlier versions of the swarm client would start and not exit on a connection failure.
By running the agent with a bogus or unreachable jenkins server your tests will see errors in the config that lead to startup failure. So you may want to reconsider 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.

I agree with you here. What you see wrong in the code about avoiding the jenkins-agent service in the case of running test environment?

Copy link
Member

Choose a reason for hiding this comment

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

The not_if here won't try to start the jenkins-agent service in test environments. But I think it should start it in all cases eve if you can't connect to the server.

Suggested change
not_if { node.chef_environment == "test" }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can try, I think that it is going to make CI to fail since the agent connection will return an error code from provisioning making the provision to stop there. Let's see if that is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's see if that is correct.

Failing now after applying the change.


  * service[jenkins-agent] action restart  * service[jenkins-agent] action restart
    
    ================================================================================
    Error executing action `restart` on resource 'service[jenkins-agent]'
    ================================================================================
    
    Mixlib::ShellOut::ShellCommandFailed
    ------------------------------------
    Expected process to exit with [0], but received '1'
    ---- Begin output of /usr/bin/systemctl --system restart jenkins-agent ----
    STDOUT: 
    STDERR: Job for jenkins-agent.service failed because the control process exited with error code.
    See "systemctl status jenkins-agent.service" and "journalctl -xe" for details.
    ---- End output of /usr/bin/systemctl --system restart jenkins-agent ----
    Ran /usr/bin/systemctl --system restart jenkins-agent returned 1

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'm going to revert the change to make CI happy. Ticketed in #7 in the case we find time to debug and solve it.

@j-rivero j-rivero merged commit c62eb64 into latest Jan 21, 2021
@j-rivero j-rivero deleted the latest_agent branch January 21, 2021 15:48
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.

2 participants