Skip to content

Fixing jenkins pipeline & tests#223

Merged
frisso merged 1 commit intopolycube-network:masterfrom
michelsciortino:ms-fixing-jenkins-tests
Oct 26, 2019
Merged

Fixing jenkins pipeline & tests#223
frisso merged 1 commit intopolycube-network:masterfrom
michelsciortino:ms-fixing-jenkins-tests

Conversation

@michelsciortino
Copy link
Copy Markdown
Contributor

@michelsciortino michelsciortino commented Oct 16, 2019

Docker base image is now built only triggered by a cron at 2:00 am (everyday) to reduce the building time of the pipeline.
Mounting volumes of the polycubed image have been changed:

  • /proc is now mounted under /host/proc to have a complete visibility of the namespaces created outside of the container.

Solving iptables tests:
Docker manipulates the iptables policy of the node during his installation and this makes the kernel drop the packets forwarded between different network namespaces; to solve this, sudo iptables -P FORWARD ACCEPT must be run on the host before running tests in order avoid the kernel drops packets.

Comment thread CI/JenkinsfileTestCleanInstance
Comment thread CI/JenkinsfileTestCleanInstance
Comment thread CI/JenkinsfileTestCleanInstance Outdated
@michelsciortino michelsciortino force-pushed the ms-fixing-jenkins-tests branch 2 times, most recently from d8e8927 to a6b2655 Compare October 23, 2019 13:42
@michelsciortino michelsciortino marked this pull request as ready for review October 23, 2019 13:45
@michelsciortino michelsciortino requested a review from a team as a code owner October 23, 2019 13:45
@palexster palexster changed the title WIP: fixing jenkins tests Fixing jenkins tests Oct 23, 2019
Comment thread Dockerfile
ADD src/components/k8s/pcn_k8s/init.sh /init.sh

CMD ["polycubed"]
CMD ["nsenter","--mount=/host/proc/1/ns/mnt","polycubed"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please add a comment saying what is this about?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

function test_tcp {
sudo ip netns exec ns2 netcat -l -w 5 $tcp_port&
sleep 2
sleep 5
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be possible to define a constant that is valid for all the tests, so that we don't have to change it manually throughout the entire file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this should be done in another PR as this change should affect every service's tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good point. Leave this as is.

@frisso
Copy link
Copy Markdown
Contributor

frisso commented Oct 24, 2019

@michelsciortino Thanks for this long awaited patch!
Two minor comments: can you add what you said in the commit description also in the proper location of the script files, so that we know immediately the reasons of the new commands?

 /proc is now mounted under /host/proc to have a complete visibility of the namespaces created outside of the container.

 sudo iptables -P FORWARD ACCEPT must be run before running test in order avoid the kernel drops

@michelsciortino
Copy link
Copy Markdown
Contributor Author

michelsciortino commented Oct 24, 2019

@frisso

can you add what you said in the commit description also in the proper location of the script files

/proc is now mounted under /host/proc to have a complete visibility of the namespaces created outside of the container.

I'v added this as a comment in the jenkinksfile

sudo iptables -P FORWARD ACCEPT must be run before running test in order avoid the kernel drops

This command is not present in the scripts as it must be run on the host;
I've added a better explanation on the PR description

@michelsciortino michelsciortino changed the title Fixing jenkins tests Fixing jenkins pipeline & tests Oct 24, 2019
@michelsciortino michelsciortino force-pushed the ms-fixing-jenkins-tests branch from 203be2b to 60c59b5 Compare October 24, 2019 15:12
@frisso frisso self-requested a review October 25, 2019 05:28
Copy link
Copy Markdown
Collaborator

@palexster palexster left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks @michelsciortino for this!

Adding croned stage 'Build Base Image'
Adding volumes to docker run command
Added nsenter in dockerfile CMD
Killing polycubed docker in Clean Instance stage
Adding /proc to Clean Instance & Same Instance Test stages + fix &policubed command in Ip Tables Tests stage
Adding docker stop polycubed at the begin of each test stage
Adding docker system prune --all --force at begin of each test stage
Wrapping polycube kill command in tests/helpers_tests.bash
Cron now triggers the pipeline only when the current branch is 'master' + removing unused labels
Removing pcn-iptables tag in iptables test stage
Increasing sleep time in src/services/pcn-nat/test/test_tcp_snat.sh
Fixing Slack notification for SUCCESS status
Adding conditional images building (images are built only if not already pushed to docker hub)
Removing dash in polycube-network
@michelsciortino michelsciortino force-pushed the ms-fixing-jenkins-tests branch from 60c59b5 to c8e50ec Compare October 25, 2019 15:40
@frisso frisso merged commit 474faf2 into polycube-network:master Oct 26, 2019
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.

3 participants