Bug 231334 - 12-ALPHA's make installworld DESTDIR=/mnt/current fails due to improper ntpd user check of /etc/passwd file
Summary: 12-ALPHA's make installworld DESTDIR=/mnt/current fails due to improper ntpd ...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Ian Lepore
URL:
Keywords: needs-qa, patch
Depends on:
Blocks:
 
Reported: 2018-09-13 03:43 UTC by ota
Modified: 2019-01-08 14:34 UTC (History)
6 users (show)

See Also:


Attachments
Proper DESTDIR/etc validation checks (1.22 KB, patch)
2018-09-23 01:49 UTC, ota
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ota 2018-09-13 03:43:43 UTC
Even though DESTDIR is specified, make installworld looks into /etc/passwd for ntpd user instead of ${DESTDIR}/etc/passed and fails to continue.

I had 11.2-RELEASE running and had mounted 12-ALPHA release under /mnt/current while I mounted 12-ALPHA#'s /usr/src and /usr/obj.
$ make installworld -C /usr/src DESTDIR=/mnt/current

That failed even though I had ntpd user setup under DESTDIR.

% grep ntpd /mnt/current/etc/passwd
ntpd:*:123:123:NTP Daemon:/var/db/ntp:/usr/sbin/nologin

I do not have ntpd user /etc/passwd as it is 11.2-RELEASE does not require.

I lost the exact message but the one says to look into /usr/src/UPDATES about ntpd user account.
Comment 1 Ian Lepore freebsd_committer 2018-09-13 13:42:24 UTC
(In reply to ota from comment #0)
This problem will not happen if you follow the correct update procedure, including the mergemaster -Fp (pre-world) step detailed as step [5], line 1700 in UPDATING.  Very often the pre-world mergemaster can be skipped, but not when new user ids are added.
Comment 2 commit-hook freebsd_committer 2018-09-13 15:16:39 UTC
A commit references this bug:

Author: ian
Date: Thu Sep 13 15:16:05 UTC 2018
New revision: 338648
URL: https://svnweb.freebsd.org/changeset/base/338648

Log:
  If a user skips the pre-world mergemaster, an installworld check
  notices the missing ntpd user and refers to UPDATING. This change makes
  it more clear which aspect of UPDATING is important for the ntpd change.

  PR:		231334
  Approved by:	re (gjb)

Changes:
  head/UPDATING
Comment 3 ota 2018-09-14 02:15:13 UTC
(In reply to Ian Lepore from comment #1)

For "make installworld DESTDIR=/mnt/current" case,
"mergemaster -Fp -D /mnt/current" needs to be run instead of "mergemaster -Fp"

The problem is /usr/src/Makefile,

.for uid in ${CHECK_UIDS}
        @if ! `id -u ${uid} >/dev/null 2>&1`; then \
                echo "ERROR: Required ${uid} user is missing, see /usr/src/UPDAT
ING."; \
                false; \
        fi
.endfor

This checks if a user exists on the host server and "not on the destination of installworld."

$ uname -r
11.2-RELEASE-p2

$ id -u ntpd
id: ntpd: no such user

$ svn info /usr/src
Path: /usr/src
Working Copy Root Path: /usr/src
URL: https://svn0.us-east.freebsd.org/base/head
Relative URL: ^/head
Repository Root: https://svn0.us-east.freebsd.org/base
Repository UUID: ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
Revision: 338677
Node Kind: directory
Schedule: normal
Last Changed Author: mmacy
Last Changed Rev: 338677
Last Changed Date: 2018-09-13 21:30:05 -0400 (Thu, 13 Sep 2018)

$ grep ntpd /mnt/current/etc/passwd
ntpd:*:123:123:NTP Daemon:/var/db/ntp:/usr/sbin/nologin

$ make installworld -C /usr/src DESTDIR=/mnt/current
...
ERROR: Required ntpd user is missing, see /usr/src/UPDATING.



Perhaps, this needs to be like

.for uid in ${CHECK_UIDS}
        @if `awk -F: '"${uid}" == $$1{rc=1}END{exit rc}' ${DESTDIR}/etc/passwd`; then \
                echo "ERROR: Required ${uid} user is missing, see /usr/src/UPDAT
ING."; \
                false; \
        fi
.endfor

I'm not sure for NIS based cases...
Comment 4 ota 2018-09-23 01:49:34 UTC
Created attachment 197388 [details]
Proper DESTDIR/etc validation checks

This fixes make installworld DESTDIR=<path> of 12-release with non-12-release host.

I tested on 11.2-RELEASE server to perform installation of 12-release with DESTDIR.
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2018-09-26 03:44:36 UTC
Assign to original committer of ntp uid additions [1] to review the DESTDIR patch for validity and resolution

[1] base r336525
    base r336526
    base r336562
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2018-09-26 03:45:32 UTC
CC Bryan for additional build system expertise
Comment 7 Andrew 2018-10-26 20:53:46 UTC
Here is my trace.

After successful freebsd-update from 11.2-RELEASE to 12.0-BETA1 
and make -j96 buildworld.

[root@nas1 /usr/src]# mergemaster -p

*** Creating the temporary root environment in /var/tmp/temproot
 *** /var/tmp/temproot ready for use
 *** Creating and populating directory structure in /var/tmp/temproot



*** Beginning comparison

 *** Temp ./etc/group and installed have the same Id, deleting
 *** Temp ./etc/master.passwd and installed have the same Id, deleting

*** Comparison complete

*** /var/tmp/temproot is empty, deleting

time make  installworld  
make[1]: "/usr/obj/usr/src/amd64.amd64/toolchain-metadata.mk" line 1: Using cached toolchain metadata from build at nas1.ioffe.ru on Fri Oct 26 22:22:24 MSK 2018
ERROR: Required ntpd user is missing, see /usr/src/UPDATING.
*** Error code 1

Stop.
make[1]: stopped in /usr/src
*** Error code 1

Stop.
make: stopped in /usr/src

real	0m0.140s
user	0m0.069s
sys	0m0.079s
[root@nas1 /usr/src]# grep ntpd /etc/passwd 
ntpd:*:123:123:NTP Daemon:/var/db/ntp:/usr/sbin/nologin
[root@nas1 /usr/src]# id -u ntpd
id: ntpd: no such user

Just rebuild passwd by vipw and all OK.
Comment 8 Bryan Drewery freebsd_committer 2018-10-26 20:58:14 UTC
(In reply to ota from comment #4)
> Created attachment 197388 [details]
> Proper DESTDIR/etc validation checks
> 
> This fixes make installworld DESTDIR=<path> of 12-release with
> non-12-release host.
> 
> I tested on 11.2-RELEASE server to perform installation of 12-release with
> DESTDIR.

A quick overview I think the DESTDIR changes make sense. Though I don't like
the grepping involved here. For mtree -N is being used to read the password
database but then we fallback on grep later. Is there a proper tool that can
be used to read the database?
Comment 9 Bryan Drewery freebsd_committer 2018-10-26 20:59:15 UTC
By the way I don't think this is a regression in 12 per say. Its perhaps just more
noticeable due to the new ntpd user. Adding ntpd on the system is an easy workaround.
Comment 10 ota 2018-10-28 03:16:05 UTC
(In reply to Bryan Drewery from comment #9)

No, it isn't a regression.
But it is a problem existed all times.

Because of 12-ALPHA bug, https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231291, and missed reinstallkernel on 2nd try, my system broke completely and required booting from CDROM with older release.

Of course, CDROM didn't like writing ntpd user account and I had to fix installkernel to boot the system again.
Comment 11 Albaro Pereyra 2019-01-06 04:01:29 UTC
I have an upgrade script that I put together based on best practices and have received this error.

Can someone help me debug it?

userinputVersion=12.0.0;
printf "About to remove old source files.\n";                                                                                                                   
rm -rf /usr/src;                                                                                                                                                
rm -rf /usr/obj;                                                                                                                                                
mkdir /usr/src;                                                                                                                                                 
printf "Downloading and preparing new source files.\n";                                                                                                                                                                                                         
svn checkout https://svn0.us-west.freebsd.org/base/release/"${userInputVersion}"/ /usr/src;                                                                     
svn up /usr/src;                                                                                                                                                
cd /usr/src;                                                                                                                                                    
make buildworld && make installworld && make buildkernel KERNCONF=GENERIC && make installkernel KERNCONF=GENERIC;                                                                                                                                                                          
mergemaster -Ui;                                                                                                                                                                                                                                                                                                                  
mv "${cron_tab_file}" "${cron_tab_file}".bak                                                                                                                    
dir=$(dirname $0);                                                                                                                                              
printf "%s\n" "@reboot      ${dir}/cronScripts/rebootCleanUpgrade.sh" >> "${cron_tab_file}";                                                                    
shutdown -r now;                                                                                                                                                
exit 0;

Thanks
Comment 12 przzz 2019-01-08 14:34:02 UTC
(In reply to Albaro Pereyra from comment #11)

What error are you received?