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

choose build pod by name AND namespace #15575

Conversation

enoodle
Copy link

@enoodle enoodle commented Jul 17, 2017

This will ensure that the ContainerBuildPod associated with a ContainerGroup is from the same namespace.

Tests are in ManageIQ/manageiq-providers-openshift#36 due to the inexistance of tests for this file in this repo.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1474076

@enoodle enoodle force-pushed the namespace_container_build_pod_identification branch from f3b2c4c to 9fb39d3 Compare July 17, 2017 13:33
@enoodle
Copy link
Author

enoodle commented Jul 17, 2017

@cben @moolitayer PTAL at this one liner
@miq-bot add_label bug

@miq-bot miq-bot added the bug label Jul 17, 2017
@miq-bot
Copy link
Member

miq-bot commented Jul 17, 2017

Checked commit enoodle@9fb39d3 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@moolitayer
Copy link

moolitayer commented Jul 17, 2017

@enoodle is there a ticket associated with this bug?

@@ -157,7 +157,8 @@ def save_container_groups_inventory(ems, hashes)
h[:container_node_id] = h.fetch_path(:container_node, :id)
h[:container_replicator_id] = h.fetch_path(:container_replicator, :id)
h[:container_project_id] = h.fetch_path(:project, :id)
h[:container_build_pod_id] = ems.container_build_pods.find_by(:name => h[:build_pod_name]).try(:id)
h[:container_build_pod_id] = ems.container_build_pods.find_by(:name => h[:build_pod_name],
:namespace => h.fetch_path(:project, :name)).try(:id)

Choose a reason for hiding this comment

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

It's weird that we have a namespace string and not container_project_id like everywhere else. We should probably fix that

Copy link

@moolitayer moolitayer left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@enoodle
Copy link
Author

enoodle commented Jul 17, 2017

@moolitayer This is a bug only in the backend that doesn't have an effect on the ui user experience, I am not sure if I can open a bug for that.

@enoodle
Copy link
Author

enoodle commented Jul 18, 2017

@blomquisg @Fryguy PTAL

@simon3z
Copy link
Contributor

simon3z commented Jul 21, 2017

LGTM 👍
@miq-bot assign blomquisg
cc @blomquisg

@miq-bot miq-bot assigned blomquisg and unassigned simon3z Jul 21, 2017
@simon3z
Copy link
Contributor

simon3z commented Jul 21, 2017

@miq-bot add_label fine/yes
@enoodle we want to backport this to fine, we need a BZ.

@enoodle
Copy link
Author

enoodle commented Jul 23, 2017

BZ added

@enoodle
Copy link
Author

enoodle commented Jul 26, 2017

ping @blomquisg

@blomquisg blomquisg merged commit ed83f8b into ManageIQ:master Jul 27, 2017
@blomquisg blomquisg added this to the Sprint 66 Ending Aug 7, 2017 milestone Jul 27, 2017
simaishi pushed a commit that referenced this pull request Aug 4, 2017
…identification

choose build pod by name AND namespace
(cherry picked from commit ed83f8b)

https://bugzilla.redhat.com/show_bug.cgi?id=1478568
@simaishi
Copy link
Contributor

simaishi commented Aug 4, 2017

Fine backport details:

$ git log -1
commit a0f7105f213d1429c6f9a12bda54350e8cb64074
Author: Greg Blomquist <blomquisg@gmail.com>
Date:   Thu Jul 27 09:09:39 2017 -0400

    Merge pull request #15575 from enoodle/namespace_container_build_pod_identification
    
    choose build pod by name AND namespace
    (cherry picked from commit ed83f8b65addc09c4f8f729716181c558ecb3863)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1478568

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…ild_pod_identification

choose build pod by name AND namespace
(cherry picked from commit ed83f8b)

https://bugzilla.redhat.com/show_bug.cgi?id=1478568
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.

7 participants