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

Modify boot option handling on Linux systems #182

Closed
wants to merge 0 commits into from
Closed

Modify boot option handling on Linux systems #182

wants to merge 0 commits into from

Conversation

saito-hideki
Copy link
Collaborator

@saito-hideki saito-hideki commented Apr 30, 2021

SUMMARY

Modify boot option handling on Linux systems:

  • Fixes [mount] Allow "boot" option on all Linux via "noauto" #28
  • Modified behavior to set noauto option if boot is no on Linux system
  • If opts contains noauto or defaults, the boot option will be ignored
  • If noauto is not specified, the default behavior will be auto, so boot: yes does not explicitly set auto.
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • ansible.posix.mount
ADDITIONAL INFORMATION

None

@saito-hideki saito-hideki changed the title [WIP] Modify boot option handling on Linux systems Modify boot option handling on Linux systems Apr 30, 2021
@saito-hideki saito-hideki changed the title Modify boot option handling on Linux systems [WIP] Modify boot option handling on Linux systems Apr 30, 2021
@saito-hideki saito-hideki changed the title [WIP] Modify boot option handling on Linux systems Modify boot option handling on Linux systems May 1, 2021
plugins/modules/mount.py Outdated Show resolved Hide resolved
@saito-hideki saito-hideki changed the title Modify boot option handling on Linux systems [WIP] Modify boot option handling on Linux systems May 10, 2021
@saito-hideki saito-hideki changed the title [WIP] Modify boot option handling on Linux systems Modify boot option handling on Linux systems May 10, 2021
@saito-hideki
Copy link
Collaborator Author

@quidame I have fixed it according to your advice. I would appreciate it if you could review it. Thanks!

if 'noauto' in opts:
args['warnings'].append("Ignore the 'boot' due to 'opts' contains 'noauto'.")
elif not module.params['boot']:
args['boot'] = 'no'
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the point to set it as a string rather than a boolean ?

Copy link
Collaborator Author

@saito-hideki saito-hideki May 11, 2021

Choose a reason for hiding this comment

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

Hi! thank you for the review. That was to match the return value type with Solaris. For Solaris, return value includes boot but it is string.
I have fixed it to use boolean but eventually, I think that we also need to change the type of return value for Solaris. But I think it's probably better to do it in another PR.
@quidame I would appreciate your advice on this :)

Copy link
Collaborator Author

@saito-hideki saito-hideki May 11, 2021

Choose a reason for hiding this comment

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

@quidame I have created another PR #185. This not only resolves #184, but also changes the boot parameter type to boolean in return values on the Solaris node.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#185 has been merged to main branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please rebase this branch? I guess the conflicts are because of #185 you've just mentioned.

@saito-hideki saito-hideki changed the title Modify boot option handling on Linux systems [WIP] Modify boot option handling on Linux systems May 11, 2021
@saito-hideki saito-hideki changed the title [WIP] Modify boot option handling on Linux systems Modify boot option handling on Linux systems May 11, 2021
@quidame
Copy link
Contributor

quidame commented May 22, 2021

the changelog-fragment check seems to be fixed and should pass now

@Akasurde Akasurde closed this May 24, 2021
@Akasurde Akasurde reopened this May 24, 2021
@saito-hideki
Copy link
Collaborator Author

Closing and re-opening for CI trigger.

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.

[mount] Allow "boot" option on all Linux via "noauto"
4 participants