Search domain improvements

[grenzr]

What this PR does / why we need it:

As noted in #3737, I was unable to get this feature working out of the box without:

• FEATURE: addition of computerOU switch to the realm command so that machines can join into a specific OU path in Active Directory
• BUGFIX: addition of apt-get update before custom search domain packages are installed
• BUGFIX: fix custom search domain sed replacement in acs-engine/parts/k8s/kubernetesagentcustomdata.yml (not yet working)

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

fixes #3737

Special notes for your reviewer:

If applicable:

• documentation
• unit tests
• tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

- FEATURE: addition of computerOU switch to the realm command so that machines can join into a specific OU path in Active Directory
- BUGFIX: addition of `apt-get update` before custom search domain packages are installed
- BUGFIX: fix custom search domain sed replacement in acs-engine/parts/k8s/kubernetesagentcustomdata.yml (not yet working)

[acs-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: grenzr
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: brendandburns

If they are not already assigned, you can assign the PR to them by writing /assign @brendandburns in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[msftclas]

All CLA requirements met.

[jackfrancis]

This /lgtm @CecileRobertMichon could you take a pass as well?

[CecileRobertMichon]

you will need to add this line to https://github.com/Azure/acs-engine/blob/fix-multiple-agentpools/parts/k8s/kubernetesmastercustomdatavmss.yml#L384 as well

[grenzr]

ok thanks, will do - however my issue is that the sed substitutions do not occur during bootstrap of each machine.
see comment in #3737 for further info:
#3737 (comment)

I'm not sure the kubelet service (which I believe is supposed to trigger this script) is working properly. Any idea why that might be?

[codecov]

Codecov Report

Merging #3911 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #3911      +/-   ##
==========================================
- Coverage   57.53%   57.52%   -0.02%     
==========================================
  Files         108      108              
  Lines       16747    16750       +3     
==========================================
  Hits         9635     9635              
- Misses       6341     6344       +3     
  Partials      771      771

[grenzr]

freshly rebased and added searchDomainComputerOU as requested. Just need some support on why the sed substitution script (supposedly triggered by kubelet.service) doesn't occur during machine bootstrap if possible please?

[jackfrancis]

@grenzr what does the shell script look like after kubelet starts? Specifically the value of sdComputerOU=?

(speaking to your statement that the sed replacement isn't working)

[grenzr]

@jackfrancis sorry for delay - haven't forgotten about this, had some other pressing things to deal with last week. Will get you the logs asap.

[grenzr]

@jackfrancis soo... I just rebased this branch locally with v0.22.4 and built a new acs-engine binary and tested a deployment with it (including my changes above).

It failed again with exit code 80 on the CSE.

The contents of /opt/azure/containers/setup-custom-search-domains.sh looks like this :

# cat /opt/azure/containers/setup-custom-search-domains.sh
#!/bin/bash
set -x
source /opt/azure/containers/provision_source.sh

sdName="<searchDomainName>"
sdRealmUser=$"<searchDomainRealmUser>"
sdRealmPassword=$"<searchDomainRealmPassword>"
sdComputerOU=$"<searchDomainComputerOU>"
ucDomainName=$(echo "${sdName}" | tr /a-z/ /A-Z/)

computerOUSwitch=""
if [[ ! -z "${sdComputerOU}" ]]; then
  computerOUSwitch="--computer-ou='${sdComputerOU}'"
fi

echo "  dns-search ${sdName}" >> /etc/network/interfaces.d/50-cloud-init.cfg
systemctl_restart 20 5 10 restart networking

retrycmd_if_failure 10 5 120 apt-get update
retrycmd_if_failure 10 5 120 apt-get -y install \
  realmd \
  sssd \
  sssd-tools \
  samba-common \
  samba \
  python2.7 \
  samba-libs \
  packagekit

echo "${sdRealmPassword}" | \
  realm join -U "${sdRealmUser}@${ucDomainName}" ${ucDomainName} ${computerOUSwitch}

ie. none of the substitute occurred at all.

/etc/systemd/system/kubelet.service contains:

[Unit]
Description=Kubelet
ConditionPathExists=/usr/local/bin/kubelet
# Required

[Service]
Restart=always
EnvironmentFile=/etc/default/kubelet
SuccessExitStatus=143
ExecStartPre=/bin/bash /opt/azure/containers/kubelet.sh
ExecStartPre=/bin/mkdir -p /var/lib/kubelet
ExecStartPre=/bin/mkdir -p /var/lib/cni
ExecStartPre=/bin/bash -c "if [ $(mount | grep \"/var/lib/kubelet\" | wc -l) -le 0 ] ; then /bin/mount --bind /var/lib/kubelet /var/lib/kubelet ; fi"
ExecStartPre=/bin/mount --make-shared /var/lib/kubelet
# This is a partial workaround to this upstream Kubernetes issue:
#  https://github.com/kubernetes/kubernetes/issues/41916#issuecomment-312428731
ExecStartPre=/sbin/sysctl -w net.ipv4.tcp_retries2=8
ExecStartPre=-/sbin/ebtables -t nat --list
ExecStartPre=-/sbin/iptables -t nat --list
ExecStart=/usr/local/bin/kubelet \
        --enable-server \
        --node-labels="${KUBELET_NODE_LABELS}" \
        --v=2 \
        --volume-plugin-dir=/etc/kubernetes/volumeplugins \
        $KUBELET_CONFIG $KUBELET_OPTS \
        $KUBELET_REGISTER_NODE $KUBELET_REGISTER_WITH_TAINTS

[Install]
WantedBy=multi-user.target

So when kubelet starts its supposed to call this first: /opt/azure/containers/kubelet.sh (which contains the sed lines we care about at the bottom).

systemctl list-unit-files shows kubelet.service disabled so I'm guessing this service never starts in order to do the substitutions.

However, the custom-search-domains.sh is called, but fails (as expected) with :

Processing triggers for libc-bin (2.23-0ubuntu10) ...
Processing triggers for systemd (229-4ubuntu21.4) ...
Processing triggers for ureadahead (0.100.0-19) ...
Processing triggers for ufw (0.35-0ubuntu2) ...
Processing triggers for dbus (1.10.6-1ubuntu3.3) ...
+ '[' 0 -eq 0 ']'
+ break
+ echo Executed '"apt-get' -y install realmd sssd sssd-tools samba-common samba python2.7 samba-libs 'packagekit"' 1 times
Executed "apt-get -y install realmd sssd sssd-tools samba-common samba python2.7 samba-libs packagekit" 1 times
+ echo '<searchDomainRealmPassword>'
+ realm join -U '<searchDomainRealmUser>@<SEARCHDOMAINNAME>' '<SEARCHDOMAINNAME>' '--computer-ou='\''<searchDomainComputerOU>'\'''
realm: No such realm found

Isn't the kubelet service supposed to be enabled or if not, when is it? If not at boot then it looks like its happening too late for the custom search domain script.

[grenzr]

heh... it can't start on boot if the service isn't installed yet, eh? :)
looks like its hanging onto the ensureKubelet() function, which is called by /opt/azure/containers/provision.sh..

Yeah looks like ensureKubelet is called too late as it happens well after attempts to run $CUSTOM_SEARCH_DOMAIN_SCRIPT .. I think here lies the problem..

[grenzr]

freshly rebased from master, and have now provided a PoC solution in the latest commit, which appears to bring up a working cluster now.

[jackfrancis]

@grenzr let's undo the last commit, that's not the way we want to move this forward (it has wide ranging consequences)

Question: is the new computerOU configuration optional? Or a new requirement for this functionality?

[grenzr]

@jackfrancis sure, I'll remove it shortly. Was only there temporarily for demo. Do you have any thoughts on a better alternative?

Yes, the computerOU I believe is optional currently, as I wanted to maintain backwards compatibility with the existing implementation (although I'm not sure it was working properly to begin with?)

This piece only adds the option to the realm command if its non-empty:

computerOUSwitch=""
if [[ ! -z "${sdComputerOU}" ]]; then
  computerOUSwitch="--computer-ou='${sdComputerOU}'"
fi

and the defaultValue for it is an empty string in kubernetesparams.t

Anything else I've missed?

I haven't run any ginkgo tests across it locally yet.

[grenzr]

PoC commit removed

[CecileRobertMichon]

@grenzr thank you for your contribution! We've relocated to https://github.com/Azure/aks-engine/. What is the status of this PR? I am closing this one as we are no longer accepting PRs for acs-engine but love to see this go through in aks-engine. Please let me know if you are willing to reopen the PR at https://github.com/Azure/aks-engine/pulls. Thanks!