Skip to content

(PUP-1628) Fix mount provider for AIX#3740

Closed
hunner wants to merge 5 commits intopuppetlabs:stablefrom
hunner:fix/stable/aix_mount
Closed

(PUP-1628) Fix mount provider for AIX#3740
hunner wants to merge 5 commits intopuppetlabs:stablefrom
hunner:fix/stable/aix_mount

Conversation

@hunner
Copy link
Contributor

@hunner hunner commented Mar 19, 2015

The way that AIX tracks its mounted filesystems is different than other
unix or linux operatingsystems. AIX uses /etc/filesystems and has
entirely different filesystem options.

This commit takes advantage of two parsedfile hooks, post_parse and
to_line, to read & generate /etc/filesystem entries. Parsedfile was
not created with arbitrary-ordering in mind, and any /etc/filesystem
entries will have their attributes reordered by to_line

Options in AIX without related properties in Puppet's mount type are
placed under the "options" property.

@ahpook
Copy link
Contributor

ahpook commented Mar 19, 2015

WOW!! Awesome @hunner -- I'm super happy to see this addressed.

One comment, can you change the commit message to be prefixed with the public-facing ticket (PUP-1628) rather than the internal-only PE ticket? That way anyone can see the history not just PL folks.

@hunner hunner force-pushed the fix/stable/aix_mount branch 2 times, most recently from 9e0df65 to 779cf2d Compare March 19, 2015 23:35
@hunner hunner changed the title (WIP) (PE-8657) Fix mount provider for AIX (PUP-1628) Fix mount provider for AIX Mar 19, 2015
@stahnma
Copy link
Contributor

stahnma commented Mar 20, 2015

All the 👍 s. 😄

@puppetcla
Copy link

CLA signed by all contributors.

@hunner
Copy link
Contributor Author

hunner commented Mar 20, 2015

Limitations:

  • It will reorder the filesystem attributes to a specific order (dev, nodename, vfs, then the rest alphabetic)
  • Comments embedded in a filesystem attribute list will cause the following ones to be dropped (but between filesystem entries is fine)
  • The options property is comma separated (fine) and must be in the order from the first bullet point, otherwise it will always try to update the value
  • The options property is overloaded with all of AIX's extra attributes; perhaps adding new properties to the type would be better

@helperton
Copy link

Hello Hunner, I've patched my parsed.rb and mount.rb with these changes. What I'm seeing is that for "nodename" in /etc/filesystems the value is the entire string which is defined in the manifest "hostname:device" where it should just be "hostname".

Here is the string from the "nodename" property of the mount resource:

'/someplace':
device: 'nfsserver:/oracle_depot/mnt'

So our /etc/filesystems look like this now:

Wrong:

/someplace:
dev = /someplace <-- should be right side of a split on ":" of "device")
nodename = nfsserver:/oracle_depot/mnt <- should be left side of a split on ":" of "device")
vfs = nfs
options = nosuid,ro,soft

Should look like:

/someplace:
dev = "/oracle_depot/mnt" < -- correct
vfs = nfs
nodename = nfsserver <--- correct
mount = true
options = bg,soft,intr
account = false

@hunner hunner force-pushed the fix/stable/aix_mount branch 2 times, most recently from 7b05b6f to b0f17a4 Compare March 21, 2015 00:13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be turned into logic, not a scary regex.

@helperton
Copy link

Ok, so this works great. Could you make one cosmetic change and put in one blank before each entry you put in /etc/filesystems?

@hunner
Copy link
Contributor Author

hunner commented Mar 23, 2015

@helperton Blank lines and comments are preserved across updates, so if you have blank lines before each entry, they should stay there. I'd rather not enforce blank lines before every entry, as some users may not prefer that style. Is that okay?

@helperton
Copy link

I think it's a requirement by IBM

https://www.ibm.com/developerworks/community/wikis/home?lang=en#!/wiki/Power+Systems/page/How+to+change+order+of+fs+mounts+on+AIX+V5.3

Filesystems are mounted at AIX boot time in the order they appear in /etc/filesystems. Filesystems are added to /etc/filesystems in the order they are defined. The easiest way to change the order in which filesystems are mounted is to edit /etc/filesystems and change the ordering of the filesystem stanzas. (But be careful to maintain a blank line between each stanza. If blank lines are removed, some filesystems won't get mounted at boot time!)

@kylog
Copy link

kylog commented Mar 23, 2015

@helperton I read that three times and I'm still somewhat baffled. What do the blank lines signify? That advice says not to remove blank lines but not what they mean. It sounds like this is not a cosmetic change so I'm curious how it actually works.

@helperton
Copy link

Ya, I thought it was cosmetic until I found that article. I was suspicious that all of our /etc/filesystems have that blank line, then when I looked at examples of /etc/filesystems on the internet I found those too, have the blank line. Then I found the IBM article.

@helperton
Copy link

Here's another article which talks about the blank line (still doesn't answer your question).

http://bio.gsi.de/DOCS/AIX/2x/faqssoft.html

Looks like an IBM/AIX thing. They use this same format in other files as well.

This one too mentions no blank line being a problem:

http://ps-2.kev009.com/tl/techlib/qna/sfam/html/AC/AC3400L.htm

@kylog
Copy link

kylog commented Mar 23, 2015

I can't find anything about it, but the examples I found have a blank line between stanzas. So probably better safe than sorry ....

Also in the surprising-to-a-non-AIX-admin department:

Note: The asterisk (*) is the comment character used in the /etc/filesystems file.

which I found in http://www-01.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.aix.files/filesystems.htm

@melissa melissa added the Unix label Mar 24, 2015
@hunner
Copy link
Contributor Author

hunner commented Mar 24, 2015

@helperton Oooo, okay. I'll update the PR with blank lines then. Thanks!

hunner added 4 commits March 24, 2015 15:37
The way that AIX tracks its mounted filesystems is different than other
unix or linux operatingsystems. AIX uses /etc/filesystems and has
entirely different filesystem options.

This commit takes advantage of two parsedfile hooks, `post_parse` and
`to_line`, to read & generate /etc/filesystem entries. Parsedfile was
not created with arbitrary-ordering in mind, and any /etc/filesystem
entries will have their attributes reordered by `to_line`

Options in AIX without related properties in Puppet's mount type are
placed under the "options" property.

The specs I wrote for instances work, but there are many other failures
when running specs on AIX. This PR doesn't include specs for `to_line`
The regex worked, but any comments or blank lines in the middle of a
stanza would end the stanza parsing resulting in a malformatted file.
Plus it was not easy to read or debug.

This updates the "parser" to use simple state when parsing the file and
does not barf on comments or blank lines in the middle of stanzas
(though they will be moved to the end of the stanza to avoid edge
cases on file regeneration).
Also handle trailing spaces on the resource name, and cases when device
is malformatted nfs mounts
@hunner hunner force-pushed the fix/stable/aix_mount branch from 1b5671e to 1fb6f56 Compare March 24, 2015 23:12
@hunner
Copy link
Contributor Author

hunner commented Mar 25, 2015

Newlines updated. The only issue I've seen still occurring is the mount -o remount <nfs mountpoint> causing failures after updating an NFS entry. It seems that it must be either remount,ro or remount,rw to work correctly.

The mount type previously just ran an unmount & mount instead of a remount, but this PR changes the default to do remount... How should this be solved?

  • Change back to unmount/remount for all filesystem entries
  • Leave with remount; this error is PEBKAC
  • Make the option default to unmount/mount for NFS volumes and remount for non-NFS volumes
  • Other

@helperton
Copy link

Ok, so it did fix the newline issue, but we now seem to have some issue with the option ordering and it saying that it's updated the options, but it's not actually reflected in /etc/filesystems.

During the puppet run:

Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Loading facts
Info: Caching catalog for somehost.domain.com
Info: Applying configuration version '0cfe3f36a46cb4ddc81f790626ffbf793245f2d3'
Notice: /Stage[main]/Puppet_nfs/Mount[/nfs/blah]/options: options changed 'bg,intr,soft' to 'bg,soft,intr'
Info: Computing checksum on file /etc/filesystems
Info: /Stage[main]/Puppet_nfs/Mount[/nfs/blah]: Scheduling refresh of Mount[/nfs/blah]
Info: Mount/nfs/blah: Remounting
Notice: /Stage[main]/Puppet_nfs/Mount[/nfs/blah]: Triggered 'refresh' from 1 events
Info: /Stage[main]/Puppet_nfs/Mount[/nfs/blah]: Scheduling refresh of Mount[/nfs/blah]
Notice: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/aix]/options: options changed 'bg,intr,soft' to 'bg,soft,intr'
Info: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/aix]: Scheduling refresh of Mount[/nfs/dep/aix]
Info: Mount/nfs/dep/aix: Remounting
Notice: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/aix]: Triggered 'refresh' from 1 events
Info: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/aix]: Scheduling refresh of Mount[/nfs/dep/aix]
Notice: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/oracle]/options: options changed 'bg,intr,soft' to 'bg,soft,intr'
Info: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/oracle]: Scheduling refresh of Mount[/nfs/dep/oracle]
Info: Mount/nfs/dep/oracle: Remounting
Notice: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/oracle]: Triggered 'refresh' from 1 events
Info: /Stage[main]/Puppet_nfs/Mount[/nfs/dep/oracle]: Scheduling refresh of Mount[/nfs/dep/oracle]
Notice: Finished catalog run in 1.17 seconds

It keeps saying that it's changing the options, but it doesn't actually change them in the file, so the next puppet run outputs the exact same thing again.

@hunner
Copy link
Contributor Author

hunner commented Mar 25, 2015

@helperton Okay. I was sorting the list of options returned from /etc/filesystem (which is why it returned bg,intr,soft even though that wasn't the actual order). I removed that sort.

However, some "options" (account, boot, check, free, mount, size, type, vol, log, and quota) are passed to the mount type via the options parameter before being separated into their respective lines, and must be alphabetically sorted at the end of the comma-separated list to avoid this.

@helperton
Copy link

Bravo! Works.

@stahnma
Copy link
Contributor

stahnma commented Mar 27, 2015

After another review, I'm still happy with this. 👍

@melissa
Copy link
Contributor

melissa commented Mar 30, 2015

@hunner if this is going to get into Puppet 3.8, could you retarget this pull request against the 3.x branch? The stable branch now corresponds to 4.0 and master to 4.1.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants