Skip to content

Conversation

@mrIncompetent
Copy link
Contributor

What this PR does / why we need it:
This PR will:

  • Remove kubeadm & remove necessary deb packages
    • In theory kubeadm is nice, but we maintain coreos already different and had to introduce some special logic to cope with kubeadm changes (default file - dependency issues in the repos)
  • Provide template functions for all kinds of common flags / configuration we use in all operating systems. This will ensure that we'll always use the same flags on all systems - modifications can be done via a systemd-drop-in

cp /etc/sysconfig/kubelet-overwrite /etc/sysconfig/kubelet
# Download all required binaries
/opt/bin/download_binaries
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we simply remove the shebang from the downloadBinariesScript template and embed it here with a indent 4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inlined download script with 0221895

Copy link
Contributor

@alvaroaleman alvaroaleman left a comment

Choose a reason for hiding this comment

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

Couple of nits, overall I actually like the change :)

net.bridge.bridge-nf-call-iptables = 1
kernel.panic_on_oops = 1
kernel.panic = 10
vm.overcommit_memory = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we set this? From the docs[0]:

When this flag is 1, the kernel pretends there is always enough
memory until it actually runs out.

[0] https://www.kernel.org/doc/Documentation/sysctl/vm.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, was there before.

But looking it up revealed: kubernetes/kubernetes#15087
I'll remove it - the kubelet will set 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.

Though this seems outdated, a kubelet fatals with:

Failed to start ContainerManager Invalid kernel flag: vm/overcommit_memory, expected value: 1, actual value: 0

if [ ! -f /opt/cni/bin/loopback ]; then
wget -O /opt/cni-plugins.tar.gz https://github.com/containernetworking/plugins/releases/download/v0.6.0/cni-plugins-amd64-v0.6.0.tgz
mkdir -p /opt/cni/bin /etc/cni/net.d
tar -xvf /opt/cni-plugins.tar.gz -C /opt/cni/bin/
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please do this and all other archive downloads to come like this:

mkdir -p /opt/cni/bin /etc/cni/net.d
curl -L https://github.com/containernetworking/plugins/releases/download/v0.6.0/cni-plugins-amd64-v0.6.0.tgz|tar -xvzC /opt/cni/bin -f -

to avoid writing an archive to disk we will never touch again?

ExecStart=/usr/local/bin/supervise.sh /usr/local/bin/setup
ExecStart=/opt/bin/supervise.sh /opt/bin/setup
- path: "/etc/profile.d/opt-bin-path.sh"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be sourced and should not be executable, hence have permissions: "0644"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ExecStart=/opt/bin/supervise.sh /opt/bin/setup
- path: "/etc/profile.d/opt-bin-path.sh"
permissions: "0755"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be sourced and should not be executable, hence have permissions: "0644"

@mrIncompetent mrIncompetent force-pushed the template-consolidation branch from 0221895 to 9a83f65 Compare October 12, 2018 20:43
@mrIncompetent mrIncompetent force-pushed the template-consolidation branch from 9a83f65 to 3210707 Compare October 12, 2018 20:45
@mrIncompetent
Copy link
Contributor Author

@alvaroaleman PTAL

@alvaroaleman
Copy link
Contributor

lgtm aside from the failing tests :)

@mrIncompetent mrIncompetent merged commit a957d3a into master Oct 13, 2018
@mrIncompetent mrIncompetent deleted the template-consolidation branch October 13, 2018 12:39
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