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

fix ci make target for dirs with spaces and netctl for kubeadm #292

Merged
merged 2 commits into from
Nov 8, 2017

Conversation

chrisplo
Copy link
Contributor

@chrisplo chrisplo commented Nov 7, 2017

No description provided.

@chrisplo chrisplo force-pushed the fix_dirs_with_spaces branch 3 times, most recently from 4bb3605 to eeb1b6f Compare November 7, 2017 16:46
@chrisplo
Copy link
Contributor Author

chrisplo commented Nov 7, 2017

build PR

1 similar comment
@chrisplo
Copy link
Contributor Author

chrisplo commented Nov 7, 2017

build PR

@chrisplo chrisplo changed the title adding quotes for space dirs WIP: adding quotes for space dirs Nov 7, 2017
@chrisplo chrisplo force-pushed the fix_dirs_with_spaces branch from 2040d74 to bb52cd4 Compare November 7, 2017 18:34
@chrisplo
Copy link
Contributor Author

chrisplo commented Nov 7, 2017

build PR

Signed-off-by: Chris Plock <[email protected]>

fixing docker run
@chrisplo chrisplo force-pushed the fix_dirs_with_spaces branch from 0571601 to 8a6ed8b Compare November 7, 2017 19:55
@chrisplo chrisplo changed the title WIP: adding quotes for space dirs WIP: fix ci make target for dirs with spaces and netctl for kubeadm Nov 7, 2017
@chrisplo
Copy link
Contributor Author

chrisplo commented Nov 7, 2017

build PR

netctl used to be added in the minimal installer, but was removed.

The binary is in the netplugin container that is run on every node, so
instead of including it in the minimal installer, extract it from the
container.

Signed-off-by: Chris Plock <[email protected]>
@chrisplo chrisplo force-pushed the fix_dirs_with_spaces branch from 8a6ed8b to 97512de Compare November 7, 2017 20:43
@chrisplo
Copy link
Contributor Author

chrisplo commented Nov 7, 2017

build PR

@chrisplo chrisplo changed the title WIP: fix ci make target for dirs with spaces and netctl for kubeadm fix ci make target for dirs with spaces and netctl for kubeadm Nov 7, 2017
Copy link
Contributor

@unclejack unclejack left a comment

Choose a reason for hiding this comment

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

The failure is related to the compressed kernel modules. I'll address it separately. This PR is ready.

LGTM

@unclejack
Copy link
Contributor

@dseevr @neelimamukiri: Can you take a look at this, please? We should merge this with two reviews.

@unclejack
Copy link
Contributor

#295 is needed to fix the CI.

mounts[5]="$src_conf_path:$container_conf_path:Z"
mounts[6]="-v"
mounts[7]="$(pwd)/contiv_cache:/var/contiv_cache:Z"
docker run --rm --net=host "${mounts[@]}" $image_name sh -c "./install/ansible/install.sh $netmaster_param -a \"$ans_opts\" $install_scheduler -m $contiv_network_mode -d $fwd_mode $aci_param $cluster_param $v2plugin_param"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍

netplugin_version=$(
sed '/contiv_network_version/!d;s/.*\: \?"\(.*\)".*/\1/' \
install/ansible/env.json)
docker rm netplugin-tmp >/dev/null 2>/dev/null || :
Copy link
Contributor

Choose a reason for hiding this comment

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

&>? 2>&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.

yeah, that's shorter,can change if the CI fails again

@@ -45,7 +45,7 @@ cp -rf scripts/generate-certificate.sh $output_dir/install
chmod +x $output_dir/install/genInventoryFile.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to quote everywhere$output_dir is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

output_dir is $(mktemp -d), which wont' have spaces . . unless TMPDIR is set and has spaces, for this PR, assuming it's not set to a dir with spaces

@unclejack
Copy link
Contributor

build PR

1 similar comment
@unclejack
Copy link
Contributor

build PR

@chrisplo chrisplo merged commit fb2b24f into contiv:master Nov 8, 2017
@chrisplo chrisplo deleted the fix_dirs_with_spaces branch November 8, 2017 18: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.

3 participants