New sys.uptime variable (Mantis #1134), and sys.systime, sys.sysday

[mvpel]

This commit grew from discussion in a pull request by @nyetsche from April 2012: #46 and Mantis 1134 from May 2012: https://cfengine.com/bugtracker/view.php?id=1134

As implemented, the $(sys.uptime) variable contains the number of minutes the system has been up and running. It uses one of various techniques depending on the OS platform (determined at compile time, not run) to find the system uptime:

• sysinfo() for Linux platforms
• kstat() for Solaris platforms
• pstat_getproc() on the init process for HP-UX
• sysctl() for BSD platforms

Any platform which doesn't have one of these methods available will default to a stat(/proc/1) to try to get the create time of the init process, which falls within a few seconds of the boot time of the system. (Even though Linux and Solaris have /proc, this method is not used on those platforms even as a fallback.)

If any of these approaches fail, the routine falls back to running the "/usr/bin/uptime" or "/bin/uptime" command, and its output is parsed into days, hours, and minutes with a PCRE regexp, and converted to a usable time for the variable. Thanks to @tzz Ted Zlatanov for the suggestion: #46 (comment)

This popen() approach is unlikely to be used very frequently - probably only if the /proc filesystem isn't mounted - but it's a decent backstop to give a system the best possible chance to set $(sys.uptime) successfully.

Two other closely related variables are introduced: $(sys.systime) and $(sys.sysday). The former is simply the result of the time() system call, which is useful in manipulating timestamps in HP-UX TCB password databases, for example, and the latter is that value divided by 86,400 - the number of seconds in a day - expressed as an integer. This is, of course, useful in the /etc/shadow file to indicate the password change date.

Three new documents are added in docs/reference/vars to describe the $(sys.uptime), $(sys.systime), and $(sys.sysday) variables.

[tzz]

Thanks for working on this, Michael.

[mvpel]

You're very welcome, Ted! It's less strenuous than mowing the lawn on a Sunday afternoon.

Dang, one more change to the regexp is needed - I set a cron job to running once a minute to log the uptime output, and it came up with this:

Nov 11 11:23:00 mphp1 syslog:  11:23am  up 2 hrs,  1 user,  load average: 0.00, 0.00, 0.00

Wait, what, "hrs?" Really? I mean... really?? Sheesh.

[mvpel]

mvpel@133699e is the commit with the corrected regexp. I'm not sure if I handled the github transaction properly here... Maybe I shouldn't have squashed before pushing?

[shaunamarie]

Hi Michael, thank you for your contributions! However, I am getting some warnings/errors when I try to compile these changes. Could you please take a look and see if you can resolve these conflicts.

Thanks!

Shauna

---

sysinfo.c:427:9: warning: format '%d' expects argument of type 'int', but argument 4 has type 'time_t' [-Wformat]
sysinfo.c:427:9: warning: format '%d' expects argument of type 'int', but argument 4 has type 'time_t' [-Wformat]
sysinfo.c:429:9: warning: format '%d' expects argument of type 'int', but argument 4 has type 'time_t' [-Wformat]
sysinfo.c:429:9: warning: format '%d' expects argument of type 'int', but argument 4 has type 'time_t' [-Wformat]

sysinfo.c:2395:5: error: implicit declaration of function 'cf_popen' [-Werror=implicit-function-declaration]
sysinfo.c:2395:15: warning: assignment makes pointer from integer without a cast [enabled by default]
sysinfo.c:2396:39: warning: pointer/integer type mismatch in conditional expression [enabled by default]
sysinfo.c:2400:9: error: implicit declaration of function 'cf_pclose' [-Werror=implicit-function-declaration]

[mvpel]

Thanks for the help Shauna - I went over the code carefully and tested with HP-UX but I don't yet have the means to test the various other platforms at my immediate disposal.

At first glance, the implicit decl warning for cf_popen/cf_pclose is rather strange, because they're in prototypes3.h, which is unconditionally included in cf3.defs.h, which is the very first LOC of sysinfo.c. And it built okay on HP-UX. I'll have to ponder that one.

The 427 & 429 can be remedied like so:

snprintf(workbuf, CF_MAXVARSIZE, "%d", (int)tloc);
...and...
snprintf(workbuf, CF_MAXVARSIZE, "%d", (int)(tloc / 86400));

It's odd, I remember having that in there. I probably fumbled something with git... Though I suppose this would be better resolved by using the PRIdMAX stuff that Mikhail is implementing for time_t values.

[mvpel]

Shauna, I've got the following for the time_t type warnings:

--- a/src/sysinfo.c
+++ b/src/sysinfo.c
@@ -32,6 +32,8 @@
 #include "vars.h"
 #include "item_lib.h"

+#include <inttypes.h>
+
 #ifdef HAVE_ZONE_H
 # include <zone.h>
 #endif
@@ -419,9 +421,9 @@ void GetNameInfo3()
     }
     else
     {
-        snprintf(workbuf, CF_MAXVARSIZE, "%d", tloc);
+        snprintf(workbuf, CF_MAXVARSIZE, PRIdMAX, tloc);
         NewScalar("sys", "systime", workbuf, cf_int);
-        snprintf(workbuf, CF_MAXVARSIZE, "%d", tloc / 86400);
+        snprintf(workbuf, CF_MAXVARSIZE, PRIdMAX, tloc / SECONDS_PER_DAY);
         NewScalar("sys", "sysday", workbuf, cf_int);
         i = GetUptimeMinutes(tloc);
         if (i != -1)

... but I can't see any avenue by which the cf_popen and cf_pclose wouldn't already be defined. They're prototyped in the same file that provides SECONDS_PER_DAY. I tested again and it still didn't warn about anything on HP-UX, and it does have -Wimplicit turned on.

Please let me know, and I'll hold the commit for the time being.

The inttypes.h include above may want to wind up in cf3.defs.h or the like, since Mikhail's going to be using PRIdMAX extensively to swap out the "%jd" formats - mvpel@750f57a#src/logging.c

I figured there's no point in checking for HAVE_INTTYPES_H since pre-C99 platforms were dropped.

[shaunamarie]

Hi Michael,

Thank you for looking into this - it looks like we have recently factored out pipes.h (5be64d0), so now we need to #include pipes.h directly wherever it is used.

[mvpel]

@shaunamarie - this should do the trick. However, I think I loused up my attempt to rebase the branch, please let me know if I need to make further adjustments.
(Now I know what I want for Hanukkah, an 64-bit Linux box... http://www.pugetsystems.com/aquarium-computer.php)

[mvpel]

Ok, it looks like I have cleaned up the mistake I made - I pulled into the sys_uptime branch instead of the master branch while I was trying to rebase sys_uptime to account for the new pipes.h file.

@dottedmag also pointed out that the pub/snprintf.c replacement function is used if the domestic snprintf is not C99 compliant, so we can use "%jd" in snprintf calls even on older HP-UX and Solaris. Should I go ahead and squash these three commits?

[mvpel]

Any update on this?

[shaunamarie]

@mvpel - I think your changes look very good, I've rebased into my own branch, and it is passing our acceptance tests but it will be nice to verify the new feature with an acceptance test before merging the changes to cfengine master. Could you please provide an acceptance test to verify the functionality of the new variables (sys.uptime, sys.systime, and sys.sysday)? Thanks!

Merged sys_uptime with my branch: https://github.com/shaunamarie/core/tree/sys_uptime

(PS The Linux aquarium is awesome :)

[mvpel]

I was thinking about acceptance tests for this functionality yesterday, in fact - would it be sufficient to just check to see if they contain numeric values instead of an unexpanded variable, or would we want to verify that the values are actually correct?

[dottedmag]

I'd provide stub versions of corresponding system functions (see the unit
test for hostname checks).

On Sat, Feb 2, 2013 at 4:53 PM, Michael Pelletier
notifications@github.comwrote:

I was thinking about acceptance tests for this functionality yesterday, in
fact - would it be sufficient to just check to see if they contain numeric
values instead of an unexpanded variable, or would we want to verify that
the values are actually correct?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/229#issuecomment-13032473.

[mvpel]

Hm... I'm having some trouble settling on the right approach for this... one of the functions involved is stat(), for instance - should I stub that out by checking for the test argument of "/proc/1" and then pass it along to the real stat() if not? But then, how do I get from the stub stat() to the real stat()...

This seems to be the right approach for HP, but I'm not sure about the others...

#if defined(__hpux)
int pstat_getproc(struct pst_status *buf, size_t elemsize, size_t elemcount, int index)
{
    assert_int_equal(elemsize, sizeof(struct pst_status));
    assert_int_equal(elemcount, 0);
    assert_int_equal(index, 1); /* init process */
    buf->pst_start = (time_t)1075950000; /* test boot time */
    return(1);
}

[mvpel]

I haven't had the opportunity to work on the acceptance tests.

[mvpel]

@shaunamarie - I merged master into my sys_uptime branch, resolving a variety of merge conflicts.

[kacf]

Thanks for the work! Superseded by #809.