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

Updated VCR from openshift repo, updated quota & volume specs #216

Merged
merged 6 commits into from
Jan 29, 2018

Conversation

cben
Copy link
Contributor

@cben cben commented Jan 15, 2018

VCR files copied from
ManageIQ/manageiq-providers-openshift#83
(can be merged independently but let's review both first).

  • Volumes: no longer rely on VCR happening to include Hawkular &
    Cassandra, test specific volumes I added to template.

  • Quotas: test current in-place update, no archiving behavior.
    (To be changed in later PR.)
    https://bugzilla.redhat.com/show_bug.cgi?id=1504560

@miq-bot add-label test, gaprindashvili/yes

@zeari @yaacov @moolitayer please review (recommend by commits)

Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

LGTM

@cben cben force-pushed the quota-tests branch 2 times, most recently from 243657c to d80a96a Compare January 24, 2018 19:48
@cben
Copy link
Contributor Author

cben commented Jan 24, 2018

@zeari @yaacov @moolitayer PTAL.
Final VCR copied from ManageIQ/manageiq-providers-openshift#83, more refactoring.
@zeari I moved fake node creation to be "part of" 1st refresh as discussed on #209 (comment)

@@ -392,13 +392,17 @@ def tag_in_category_with_description(category, description)
:match_requests_on => [:path,]) do # , :record => :new_episodes) do
EmsRefresh.refresh(@ems)
end

# fake node that should get archived on later refresh
@archived_node = FactoryGirl.create(:container_node, :name => "node", :ems_id => @ems.id)
Copy link

@moolitayer moolitayer Jan 25, 2018

Choose a reason for hiding this comment

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

I see you just moved this one around, Is @archived_node used somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #209 (comment) for benefit of moving, fits with other archived counts.
instance var unused, let me drop it 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dropped instance var

let(:container_volumes_count) { 22 }
let(:persintent_volumes_count) { 2 }
let(:container_volumes_count) { 21 }
let(:persintent_volumes_count) { 103 } # 3 from our template + minishift creates pv0001..pv0100

Choose a reason for hiding this comment

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

It would be cleaner to remove the extra 10 volumes in the future.
When someone reproduces a VCR it should be the same for minishift, origin, kubernetes etc. It would be good to assume an empty environment. Is it possible to delete the volumes in the beginning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea.

oc delete --ignore-not-found pv $(printf "pv%s " $(seq -w 0001 0100))

Do you want me to re-record?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done (implemented in ManageIQ/manageiq-providers-openshift#83, copied new VCR, count is now 3).

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 👍

cben added 5 commits January 28, 2018 23:45
Script deletes (and refresh should archive):
po/my-build-config-0-1-build
po/my-pod-0
+
po/my-build-config-1-1-build
po/my-pod-1
+
po/my-pod-2
@miq-bot
Copy link
Member

miq-bot commented Jan 28, 2018

Checked commits cben/manageiq-providers-kubernetes@ea58a4f~...e62556c with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

@cben
Copy link
Contributor Author

cben commented Jan 28, 2018

addressed review & rubocop. good to go from my side.

@moolitayer moolitayer merged commit ca27fb7 into ManageIQ:master Jan 29, 2018
@moolitayer moolitayer added this to the Sprint 78 Ending Jan 29, 2018 milestone Jan 29, 2018
@moolitayer moolitayer self-assigned this Jan 29, 2018
simaishi pushed a commit that referenced this pull request Mar 22, 2018
Updated VCR from openshift repo, updated quota & volume specs
(cherry picked from commit ca27fb7)

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

Gaprindashvili backport details:

$ git log -1
commit a8fa11f74ba56063be14e1ee9f74b11461791b02
Author: Mooli Tayer <[email protected]>
Date:   Mon Jan 29 07:48:02 2018 +0200

    Merge pull request #216 from cben/quota-tests
    
    Updated VCR from openshift repo, updated quota & volume specs
    (cherry picked from commit ca27fb701363dc6e8bb17a93c17d60b2c1785443)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1559544

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.

5 participants