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

Snapshots fail after updating to 3.8-13.04 #1296

Closed
maxhq opened this Issue Apr 27, 2016 · 23 comments

Comments

Projects
None yet
5 participants
@maxhq
Contributor

maxhq commented Apr 27, 2016

Since the update I get the following error messages from /opt/rockstor/bin/st-snapshot XX:

Traceback (most recent call last):
  File "/opt/rockstor/bin/st-snapshot", line 43, in <module>
    sys.exit(scripts.scheduled_tasks.snapshot.main())
  File "/opt/rockstor/src/rockstor/scripts/scheduled_tasks/snapshot.py", line 77, in main
    cwindow = sys.argv[2]
IndexError: list index out of range
@MFlyer

This comment has been minimized.

Member

MFlyer commented Apr 27, 2016

Hi @maxhq, that sounds strange.

On the fly workaround that could solve: edit your task and just submit it again

Please let me know if this solve it

Flyer

EDIT: Pull request follows, found missing check for sis.argv[2] not present

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Apr 27, 2016

rockstor#1296 Add control to pool_scrub and snapshot to avoid missing…
… cwindow arg - set to default run always on missing arg 2
@ScarabMonkey

This comment has been minimized.

Contributor

ScarabMonkey commented Apr 27, 2016

I can confirm: editing each task and re-submitting does indeed fix this.

@MFlyer

This comment has been minimized.

Member

MFlyer commented Apr 27, 2016

Thanks @ScarabMonkey for confirmation 😊 and sorry again for the missing check (while testing I probably didn't got that because snapshots started after i had already save tasks with new format ! )

schakrava added a commit that referenced this issue May 1, 2016

Merge pull request #1297 from MFlyer/issue#1296_extended_snapshots_sn…
…apshot.py_patch

#1296 Add control to pool_scrub and snapshot to avoid missing cwindow…
@maxhq

This comment has been minimized.

Contributor

maxhq commented May 2, 2016

Sorry but this still does not work for me (RockStor 3.8-13.06).
I opened and saved all scheduled snapshot tasks but now I get the following errors from cron (e.g. cron task /opt/rockstor/bin/st-snapshot 3 *-*-*-*-*-*):

Traceback (most recent call last):
  File "/opt/rockstor/bin/st-snapshot", line 44, in <module>
    sys.exit(scripts.scheduled_tasks.snapshot.main())
  File "/opt/rockstor/src/rockstor/scripts/scheduled_tasks/snapshot.py", line 78, in main
    if (crontabwindow.crontab_range(cwindow)): #Performance note: immediately check task execution time/day window range to avoid other calls
  File "/opt/rockstor/src/rockstor/scripts/scheduled_tasks/crontabwindow.py", line 33, in crontab_range
    hour_start = int(range_windows[0]) if range_windows[0] != '*' else 0
ValueError: invalid literal for int() with base 10: 'hp'
@schakrava

This comment has been minimized.

Member

schakrava commented May 2, 2016

pr is merged but it did not ship in .06. Planned to be part of next one coming later today.

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 2, 2016

Hi @maxhq that's a really strange one.

What does that line?? If value not equal to * then it has to be 0-x number (got via a split() so we convert it to int()).
So we have 2 options: value is * or a number (string format converted to number).

I don't understand why you get int trying to convert 'hp' ?!?
Before pull request I've tested it both with * and number (string to int), without errors.
@schakrava & @phillxnet , have you got any error??

Thanks
Flyer

EDIT: Ok, Suman was writing too with the answer 😄

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 3, 2016

@schakrava I see... btw. is there any way to see which commit belongs to an unstable release?
Maybe in your forum posts you could include a link to the corresponding Git commit?
@MFlyer Thanks for answering.

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 6, 2016

Hi @maxhq do you think we can close this one?

Thanks
Flyer

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 6, 2016

It's strange, I guess the changes are now integrated into 3.8-13.07 ?
It still does not work for me. The output of the cron jobs is still exactly like I wrote above.

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 6, 2016

Hi @maxhq that's really strange and actually I cannot test directly (dev env actually under tests for Log System on an old Rockstor ver without new tasks features). @phillxnet / @schakrava any similar matter??

Flyer

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue May 8, 2016

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 8, 2016

Hi @maxhq , finally I've moved my dev env to latest Rockstor and performed some testing to reproduce your error.

While discovering anotherone (5884b49), I also confirmed that just editing a task and submitting it autofix every problem (auto add the "run always" code), so your report is a kind of mistery because actually my *-*-*-*-*-* value runs without errors!?!?

schakrava added a commit that referenced this issue May 10, 2016

Merge pull request #1310 from MFlyer/issue#1296_failing_snapshots_aft…
…er_update_to_3.8-13.04

#1296 Edit to sys.argv len, to be > 2 and not 1
@schakrava

This comment has been minimized.

Member

schakrava commented May 10, 2016

@maxhq

Maybe in your forum posts you could include a link to the corresponding Git commit?

I like the idea, thanks for suggesting it. Will do going forward.

@schakrava schakrava added the bug label May 11, 2016

@schakrava schakrava added this to the Looney Bean milestone May 11, 2016

@schakrava

This comment has been minimized.

Member

schakrava commented May 11, 2016

Closed by #1310

@schakrava schakrava closed this May 11, 2016

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 24, 2016

I hope I am not annoying you... but... the problem (as written here) is still there on my machine with Rockstor 3.8-13.12.
Same error, although I edited and saved all cron jobs again. Any ideas how I can fix that?

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 24, 2016

Hi @maxhq 😕 , you're not annoying, don't worry about that.

Ok, let's check : can you list your scheduled tasks here?? Need to know for each snapshot task crontab and execution window (so i can try to reproduce that!) :)

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 24, 2016

Hi @MFlyer,

thanks a lot for investigating!
This is my /etc/cron.d/rockstortab:

SHELL=/bin/bash
PATH=/sbin:/bin:/usr/sbin:/usr/bin
MAILTO=root
MAILFROM=my@mail.local
# These entries are auto generated by Rockstor. Do not edit.
0 20 * * * root /opt/rockstor/bin/st-snapshot 4 *-*-*-*-*-*
0 14 * * * root /opt/rockstor/bin/st-snapshot 3 *-*-*-*-*-*
0 23 * * 0 root /opt/rockstor//bin/st-pool-scrub 14 *-*-*-*-*-*
0 20 * * 0 root /opt/rockstor//bin/st-pool-scrub 13 *-*-*-*-*-*
0 10 * * * root /opt/rockstor/bin/st-snapshot 11 *-*-*-*-*-*
0 10 * * * root /opt/rockstor/bin/st-snapshot 10 *-*-*-*-*-*
0 23 * * 0 root /opt/rockstor/bin/st-snapshot 9 *-*-*-*-*-*
0 20 * * * root /opt/rockstor/bin/st-snapshot 8 *-*-*-*-*-*
0 14 * * * root /opt/rockstor/bin/st-snapshot 7 *-*-*-*-*-*
0 23 * * 0 root /opt/rockstor/bin/st-snapshot 5 *-*-*-*-*-*

The Rockstor config looks like this:
bildschirmfoto_2016-05-24_18-02-41

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 24, 2016

Hi @maxhq, actually that seems ok and this is a really strange one.
This is what happens on that portion of code:
If value is * then set value to 0 (zero) <- integer
If value not * then value = string converted to integer (0 to 23 for hours)

Here is an example from a python shell trying to reproduce your error:

[root@rockstone ~]# python
Python 2.7.5 (default, Jun 24 2015, 00:41:19)
[GCC 4.8.3 20140911 (Red Hat 4.8.3-9)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> int('1')
1
>>> int('1.1')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for int() with base 10: '1.1'
>>> int('hp')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for int() with base 10: 'hp'

int('1') return an integer equal to 1 from string '1'
int('1.1') return error because converting from string '1.1' to integer (that should be instead float)
your error
int('hp') return error because trying to convert string 'hp' to an integer so, now the question: your rockstor crontab file is ok, where does that 'hp' come from?!?

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 24, 2016

@MFlyer you won't believe it - magic :-P
I had a bad feeling about the stars *-*-*-*-*-* and it was right. I logged the arguments the script snapshot.py sees.
Manually executing this gives the familiar error:

/opt/rockstor/bin/st-snapshot 4 *-*-*-*-*-*
--> ARG1: 4 | ARG2: hp-firmware-system-j06-2015.07.16-1.1.i386.rpm

This is a file on my hard disk!

But then if I execute this everything works fine: (note the quotes)

/opt/rockstor/bin/st-snapshot 4 '*-*-*-*-*-*'
--> ARG1: 4 | ARG2: *-*-*-*-*-*

So for some unknown reason cron suddenly started to do interpolation of Bash asterisks. Or maybe they are replaced because the script is executed through Bash via the Shebang line and not directly via python executable.

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 25, 2016

Hi @maxhq , happy to see this, so finally it's ok, right? 😄

If so move to this: #1290 - I'd like to have opinions from you and @ScarabMonkey too! 😉

@maxhq

This comment has been minimized.

Contributor

maxhq commented May 25, 2016

Hi @MFlyer, well I found the problem but manually editing crontab seems not like a good solution.
This currently seems to only happen on my machine. But if it can happen, I suggest the corresponding script should be changed to include quotes in the generated crontab entries.
A small change that can save others from headaches.

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 25, 2016

Hi @maxhq , I'll have some tests and probably update it.
I'd like to hear from @schakrava & @phillxnet cos' this is really odd 😕

I've done 2 running snapshot task + 1 scrub task and it was all fine (snapshots going and scrub got start-running-finished cycle without problems)

@phillxnet

This comment has been minimized.

Member

phillxnet commented May 25, 2016

@MFlyer It does look like the cron initiated shell is processing the * as a wildcard and in @maxhq 's instance returning a file name (nice find that by the way people). That file name is then parsed up to it's '-' char (the delimiter char) and hence the 'hp' which of course throws things. No news to you both I know but just wanted to articulate my reading of findings so far as just dropping in on this one and not up on how it works just yet.

It would definitely seem like there is fragility here as the intention is to not have the shell interpret those * chars and if using appropriate bracketing stops that then great. Sorry but don't yet know the ramifications of this modification. May be happening to @maxhq simply due to a matching file in the chroot environment. @maxhq is that file located in /root?

@MFlyer

This comment has been minimized.

Member

MFlyer commented May 25, 2016

Ok, manually had it running over a snapshot and no problems, but actually * is a shell wildcard (I didn't think about that while coding) so we should have a workaround to avoid @maxhq like problems ( first idea with one line mod to code: add a \ backlash before first character of crontab window string - tested and working - submitting code mod)

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue May 25, 2016

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue May 25, 2016

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