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

Restrict Vm Operating System detection for XP #17405

Merged
merged 1 commit into from
Jun 12, 2018

Conversation

aufi
Copy link
Member

@aufi aufi commented May 10, 2018

A code for setting Operating System on a Vm uses name parsing
as a fallback. There was too much generic pattern "xp" which
could mark linux Vm with name including "xp" as Windows machine,
which was incorrect and confusing for users.

Removing "xp" from Operating System patterns.

Related to https://bugzilla.redhat.com/show_bug.cgi?id=1576076

A code for setting Operating System on a Vm uses name parsing
as a fallback. There was too much generic pattern "xp" which
could mark linux Vm name including "xp" as Windows XP machines,
which was incorrect and confusing for users.

Removing "xp" from Operating System patterns.

Related to https://bugzilla.redhat.com/show_bug.cgi?id=1576076
@miq-bot
Copy link
Member

miq-bot commented May 10, 2018

Checked commit aufi@cd4e6dd with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@JPrause
Copy link
Member

JPrause commented May 10, 2018

@aufi who can be asked to review this PR. We have a blocker build coming up on Tuesday, May 15.
Also, if this can be backported, can you add the gaprindashvili/yes and fine/yes labels.

@JPrause
Copy link
Member

JPrause commented May 10, 2018

@miq-bot add_label blocker

@aufi
Copy link
Member Author

aufi commented May 11, 2018

I don't think this should be a blocker, but e.g. @agrare

@JPrause
Copy link
Member

JPrause commented May 11, 2018

Since the original BZ has been removed as a blocker,...I'll remove the label here as well.

@JPrause
Copy link
Member

JPrause commented May 11, 2018

@miq-bot remove_label blocker

@miq-bot miq-bot removed the blocker label May 11, 2018
@aufi
Copy link
Member Author

aufi commented May 22, 2018

@miq-bot add_label bug

@miq-bot miq-bot added the bug label May 22, 2018
@agrare agrare self-assigned this May 22, 2018
@agrare
Copy link
Member

agrare commented May 29, 2018

I really don't like our habit of trying to get at data from every possible place instead of being more strict, IMO the problem is falling back to the object name here instead of just using data which is supposed to contain data related to the object's operating system.

However I'm wary of removing either that line or xp from the list of OSs because it is so old and I don't think we can know what side-effects it might have for customers.

@aufi
Copy link
Member Author

aufi commented May 31, 2018

I agree on don't like this "rawly guessing" functionality and I think that removing xp line is not much beneficial, other lines seem to me even more strange.

This PR makes it at least little bit better. If core team agrees, we can remove whole OS guessing functionality..

@bdunne
Copy link
Member

bdunne commented Jun 12, 2018

I agree that the guessing is not great. @aufi Do you want to open an issue or talk article to discuss removing the "guessing" functionality?

For now I'm okay with merging as-is. I think it's unlikely that we'll see windows XP in the field since support ended April 8, 2014.

@bdunne bdunne merged commit 632d985 into ManageIQ:master Jun 12, 2018
@bdunne bdunne added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 12, 2018
@bdunne bdunne assigned bdunne and unassigned agrare Jun 12, 2018
simaishi pushed a commit that referenced this pull request Jul 11, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit c968ad633a7170c500d6c8508c833d01a1350600
Author: Brandon Dunne <[email protected]>
Date:   Tue Jun 12 10:10:39 2018 -0400

    Merge pull request #17405 from aufi/restrict_xp_os_detection
    
    Restrict Vm Operating System detection for XP
    (cherry picked from commit 632d985b0dd6b83e6fc0c5fcc392640b5179591c)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1595456

simaishi pushed a commit that referenced this pull request Aug 14, 2018
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 5559162fd5c26b04f11d0753d84f266a78905a53
Author: Brandon Dunne <[email protected]>
Date:   Tue Jun 12 10:10:39 2018 -0400

    Merge pull request #17405 from aufi/restrict_xp_os_detection
    
    Restrict Vm Operating System detection for XP
    (cherry picked from commit 632d985b0dd6b83e6fc0c5fcc392640b5179591c)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1595457

@simaishi simaishi removed the fine/yes label Aug 14, 2018
tumido pushed a commit to tumido/manageiq that referenced this pull request Aug 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants