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

Reported updates includes a list of all root directories #9

Open
deoren opened this issue Jan 21, 2018 · 12 comments
Open

Reported updates includes a list of all root directories #9

deoren opened this issue Jan 21, 2018 · 12 comments

Comments

@deoren
Copy link
Collaborator

deoren commented Jan 21, 2018

Example:

bin boot cache0 cache1 dev etc home lib lib64 lost+found media mnt opt proc root sbin selinux srv sys tmp usr var

I've not dug into the code yet to see where the problem lies.

Sampling of items in the SQLite database (/var/cache/email_updates/reported_updates.db):

sqlite> select * from reported_updates order by id desc limit 10;

607|bin boot cache0 cache1 dev etc home lib lib64 lost+found media misc mnt net opt proc root sbin selinux srv sys tmp usr var|2016-11-02 04:02:22
606|bin boot cache0 cache1 dev etc home lib lib64 lost+found media misc mnt net opt proc root sbin selinux srv sys tmp usr var|2016-11-01 04:02:28
605|bin boot cache0 cache1 dev etc home lib lib64 lost+found media misc mnt net opt proc root sbin selinux srv sys tmp usr var|2016-10-31 04:02:29
604|kernel.x86_64-2.6.18-416.el5|2016-10-29 04:02:25
603|bin boot cache0 cache1 dev etc home lib lib64 lost+found media misc mnt net opt proc root sbin selinux srv sys tmp usr var|2016-10-29 04:02:25
602|bin boot cache0 cache1 dev etc home lib lib64 lost+found media misc mnt net opt proc root sbin selinux srv sys tmp usr var|2016-10-28 04:02:26
601|bin boot cache0 cache1 dev etc home lib lib64 lost+found media misc mnt net opt proc root sbin selinux srv sys tmp usr var|2016-10-26 04:02:21
600|bin boot cache0 cache1 dev etc home lib lib64 lost+found media misc mnt net opt proc root sbin selinux srv sys tmp usr var|2016-10-25 04:02:11
599|tzdata-java.x86_64-2016h-1.el5|2016-10-22 04:02:22
598|tzdata.x86_64-2016h-1.el5|2016-10-22 04:02:22

Not only is the logic flawed that filters out applicable updates, but so is the logic that checks for previously reported updates. Entries 607 and 606 (ignoring all others) appear to be identical at first glance.

@deoren
Copy link
Collaborator Author

deoren commented Jan 21, 2018

Out of all issues migrated over, I can confirm that this is still an active issue that needs to be resolved.

deoren added a commit to deoren/email-updates that referenced this issue Jan 28, 2018
Using 'set -u' and 'set -e', both of which appear to be
best practice flags for Bash scripting.

Setting these now in an effort to help troubleshoot why
the script occasionally returns a directory listing
in place of available updates.

refs WhyAskWhy#9
@deoren
Copy link
Collaborator Author

deoren commented Jan 28, 2018

Ran the script on one of my Ubuntu 14.04 VPS boxes (2c53bec) and got this:

./email_updates.sh: line 571: SKIPPED_UPDATES[@]: unbound variable

Might not be related, but it's a start.

deoren added a commit to deoren/email-updates that referenced this issue Jan 28, 2018
Using the 'set -u' flag triggers 'unbound variable' errors
in cases where the array length may be empty. I have reworked
the checks to use what appears to be an arcane construct for
handling empty arrays properly.

refs WhyAskWhy#9
@deoren
Copy link
Collaborator Author

deoren commented Jan 28, 2018

The unbound variable error should be resolved as of db0cf4d.

deoren added a commit to deoren/email-updates that referenced this issue Jan 28, 2018
This is to force early exit if any command in a pipeline
fails instead of allowing the last exit code in the pipeline
from determining overall success/failure.

Not directly related, but this should prove useful in helping
to narrow down the specific cause of invalid patch reports
as noted on the related ticket.

refs WhyAskWhy#9
deoren added a commit to deoren/email-updates that referenced this issue Jan 28, 2018
In some cases I added guards even on code that is no longer used

refs WhyAskWhy#9
@deoren
Copy link
Collaborator Author

deoren commented Jan 28, 2018

The unbound variable error should be resolved as of db0cf4d.

Such is optimism.

@deoren deoren added this to the 0.3 milestone Jan 28, 2018
@deoren
Copy link
Collaborator Author

deoren commented Jan 29, 2018

Not that it means a whole lot, but I manually ran the latest copy of the script on two production CentOS 6 nodes without issue. I dropped the latest version in place and will wait some time to see how things shake out. If I'm unable to reproduce the issue in a few months time I will close this out.

@deoren
Copy link
Collaborator Author

deoren commented Jan 30, 2018

I deployed the current version on an Ubuntu 16.04 box and for the most part it went well enough, but I forced an update check today and got this back:

/root/email_updates.sh: line 280: previously_reported_updates[@]: unbound variable

I actually saw it happen several times, but I think the repeated values are likely due to something else (the wrapper that I was using to mass execute the command).

@deoren
Copy link
Collaborator Author

deoren commented Jan 30, 2018

Relevant code:

for previously_reported_update in ${previously_reported_updates[@]}

Full code block:

    previously_reported_updates=($(sqlite3 "${DB_FILE}" "${sql_query_match_first_field}" | cut -d '|' -f 2))

    for previously_reported_update in ${previously_reported_updates[@]}
    do
        if [[ "${VERBOSE_DEBUG_ON}" -ne 0 ]]; then
            echoerr "[I] SQL QUERY MATCH:" $previously_reported_update
        fi

        # Assume that old database entries may need multiple spaces
        # stripped from strings so we can accurately compare them
        stripped_prev_reported_update=$(sanitize_string ${previously_reported_update})

        # See if the selected patch has already been reported
        if [[ "${stripped_prev_reported_update}" == "${patch_to_check}" ]]; then
            # Report a match, and exit loop
            return 0
        fi
    done

deoren added a commit to deoren/email-updates that referenced this issue Jan 30, 2018
Another case of "unbound variable" errors

refs WhyAskWhy#9
@deoren
Copy link
Collaborator Author

deoren commented Apr 8, 2018

I've not been able to reproduce the problem since the changes in c10a604. Closing now.

@deoren deoren closed this as completed Apr 8, 2018
@deoren
Copy link
Collaborator Author

deoren commented Sep 17, 2018

I encountered the issue a few more times on some CentOS 6 boxes, but they were not running the latest version of the script so at present those results are inconclusive.

@deoren
Copy link
Collaborator Author

deoren commented Sep 18, 2018

Repeat issue today from a CentOS 6 VM.

@deoren deoren reopened this Sep 18, 2018
@deoren
Copy link
Collaborator Author

deoren commented Nov 8, 2018

Still seeing this on CentOS 6 VMs. Somewhere I'm making use of an uninitialized variable or some such programming mistake.

The good news is that I should be able to work around the issue by looking for exit code 100 from yum check-update per the man page:

       check-update
              Implemented so you could know if your machine had any updates
              that needed to be applied without running it interactively.
              Returns exit value of 100 if there are packages available for
              an update. Also returns a list of the packages to be updated
              in list format. Returns 0 if no packages are available for
              update. Returns 1 if an error occurred.  Running in verbose
              mode also shows obsoletes.

deoren added a commit to deoren/email-updates that referenced this issue Nov 8, 2018
Prior to this commit, only the filtered output from
'yum check-update' was used to determine whether
there were available updates to report. This commit
looks for the specific exit code indicated available
updates as published in the man page.

refs WhyAskWhy#9
@deoren
Copy link
Collaborator Author

deoren commented Dec 2, 2018

https://www.cyberciti.biz/tips/bash-shell-parameter-substitution-2.html

Not sure if using the approach of setting default values is the answer, but it's worth considering. I still can't help but feel I'm missing something obvious here.

Note: The changes in fec3fce / #24 seem to be working, but I'm still interested in fixing the root cause which that set of changes works around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant