[DEMO] Request and Task for physical server BIOS update #3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For seeds and import/exports, the levels explain the nesting, so there is no need to pass parent_id This is part of the Classification.parent_id initiative more factory methods that do not need to specify parent_id Remove seed update_all The categories are now properly with parent_id no need to update_all after seeding
Small refactoring to allow provider to decide which fields to clear in case "vm_changed" in provision dialog. Required to fix: https://bugzilla.redhat.com/show_bug.cgi?id=1712934
tenant_join_clause is only necessary if we are including the tenant_id_clause Before: The extra includes and requires bring back quite a few extra columns. After: only include these extra fields if we are using the fields with the where clause
miha-plesko
changed the title
|
@tadeboro this PR was created to support BIOS update demo, so that you have links to all related commits somewhere. I need to test yet, but in general this is what we need. |
only include tenant requires if in query
There is no technical difference but clean up the settings.
Change from rails :notify default since it's unlikely we'll have users setup notifications for depecation warnings.
…_behavior_but_log_in_production Use rails deprecation behavior but log in production
…_col_type Remove method get_col_type from MiqReport and its usages
One can update physical server's firmware, but the firmware has to be accessible somewhere, usually on some HTTP/FTP/SAMBA link. Furthermore, there is usually some metadata associated with the firmware to tell what kind of hardware (manufacturer, model type) is it meant for. With this commit we introduce a new model, FirmwareRegistry, that supports various kinds of metadata parsing by leveraging STI. User is supposed to create FirmwareRegistry instance via UI feeding it url+authentication and then click "Refresh Relationships" on it. ManageIQ will then invoke ``` registry.sync_fw_binaries_queue ``` which in turn will connect to the metadata repository to list available firmwares and store them as FirmwareBinary (no binary data is really stored, only HTTP/FTP/SAMBA link and some metadata). In followup PRs we will then be able to list available firmware binaries to update physical server's firmware. Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
to account for adding 425_VM Sprawl - Candidates/060_Right-Sizing Recommendations.yaml
Remove trailing spaces and convert to unix line endings
comes from ManageIQ#18784, the method exists, but not on MiqReport, only MiqExpression and adding specs for MiqReport.get_col_info - this is being used by the UI, should have tests to catch failures
MiqReport.get_col_info - fix NoMethodError parse_field_or_tag
NickL Note: Commenting out specs here that will fail, and re-implementing them as the logic is added in future commits. Most pull in specs from the ansible_tower provider repo, so they are no longer valid anyway, but can't be tested at this stage since the new model code doesn't exist yet. Also, removed the shared specs for credential.rb for the moment. This will be revisited in the future, but for now doesn't have enough merit to be included with the current implementation. Worth noting, however, is the mthods `notify_on_provider_interaction?` and `.native_ref` have been added to the credential model, since that is currently the functionality of the AnsibleTower provider (and EmbeddedAnsible by virtue of the shared code between the two), so I mirrored that here. This also fixes some test cases that were relying on the type casting of `native_ref` to work properly. The credential logic will be re-implemented and used in the future.
NickL Note: Added the specs for fixing the embedded_ansible/provider.rb since it seems to be first edited an used here. Most of the specs provided via `'ansible provider'` were not relevant for this current model, since most were surrounding the url parsing and normalizing that is done for the `default_endpoint.url`, which isn't valid for `ansible_runner` since it is run locally. Also, the factory needed to be changed to allow the provider_spec.rb to pass, which will possibly cause issues else where, but will be addressed in subsequent commits.
NickL Note: Used HIGH_PRIORITY here since that was what was used previously in the ansible_tower provider. Arguably the wrong priority, but for now this allows the specs to work as they did. Also, with this one we trade what used to check for proper refresh logic and do some light confirming of `git` logic being done properly under the hood. This is far from exhaustive tests on the subject, but I think it is a nice start without getting too into the weeds with the guts for the git logic that will most likely be removed before too long.
NickL Note: The specs mostly represent the previous spec definitions pulled in from `it_behaves_like 'ansible configuration_script'`, but updates those to properly handle the code that doesn't inherit from the ansible_tower provider code any more. That said, some behaviour has been dropped since it no longer makes sense. One change specifically that couldn't be supported as part of this change is the validations, since we would need to add those on the model side in ManageIQ (previously handled via tower), and am unsure if that is functionality we want to keep or not. The test is left there as a note (commented out).
NickL Note: Pulls in specs from the manageiq-providers-ansible_tower repo for
the job/status.rb, but makes them relevant for the new ansible_runner code.
The job/status.rb model also is a bit odd in it's implementation.
Mostly trying to keep things as close to how they worked previously,
though as mentioned in the comments, `OrchestrationStack::Status` and
`MiqTask` don't have a 1-to-1 status comparison, so some "fudging" was
necessary to keep things as DRY as possible and be able to use some of
the inherited methods from `OrchestrationStack::Status`.
Also worth noting is the change of #merge_extra_vars compared to what is
done in manageiq-providers-ansible_tower. Here, we don't return a hash
of `{:extra_vars => my_merged_vars}` since we would end up just
accessing the `:extra_vars` key directly anyway and only that in the
only caller `#run`.
Also also: The specs added to this commit are from yours truely (NickL), and
there is a decent amount here that we still need to implement, but some that my
stupid head can't figure out how to test properly currently so I think this is
"good enough for government work" for the time being. Left a lot of notes in
this one since there are clearly some spots that need to be implemented if we
are going to try and keep a parity with the existing functionality.
Using the "Tower" nomenclature is no longer valid when referring to EmbeddedAnsible. ... and yes, "nomenclature" is in my vernacular thanks to "The Big Lebowski" deal with it
This was pedantic and just was bugging me... move along...
We need the parent playbook id in order to find the filesystem path for the playbook later on.
…ord_filtering [V2V] Filter certain options in the ConversionHost#run_conversion method
…l-rugged-repos-in-specs [EmbeddedAnsible] Use rugged for git in specs
…o_authentications Encrypt Authentications become_password and auth_key_password
…tling [V2V] Modify active_tasks so that it always reloads
Add OOTB report: VM Sprawl - Right-Sizing Reccomendations
fix data type for compliance_mixin
move hashdiff reference to dependent gem
This was removed as part of 63be848 when decoupling EmbeddedAnsible from the AnsibleTower::Shared code, however, this method is needed when going to: Configuration -> Management In the UI. For now, just adding this method back in to fix things up and unblock others, but some tests along with a evaluation of the other methods not brought over should be done to determine if those methods are needed should be done as well.
…anagement_page_with_embedded_ansible Re-add EmbeddedAnsible::ConfiguredSystem#ext_management_system
Followup to ManageIQ#18868 This a development dependency that can be added individually for anyone who needs it. The problem is `hamlit` tries to require `haml` in the template [here](https://github.com/k0kubun/hamlit/blob/94b14839cd42e8b70715ba7c0caf18c2c47ec67a/lib/hamlit/template.rb#L8) and because we have `haml` in our dependency tree due to `haml_lint` (for dev), it will be able to require it. Because this causes the rails 5.1 deprecated constant to be loaded, we get this deprecation warning: ``` DEPRECATION WARNING: ActionView::Template::Handlers::Erubis is deprecated and will be removed from Rails 5.2. Switch to ActionView::Template::Handlers::ERB::Erubi instead. (called from <top (required)> at /Users/joerafaniello/Code/manageiq/config/application.rb:31) ``` Since haml_lint doesn't have a version that forces haml >= 5.0 (where this is fixed), we have to either: * add haml it as a direct dependency so we can say >= 5.0 (like this PR) * drop haml_lint entirely (so haml doesn't get in the dependency tree) * ignore/live with this warning whenever you run tests or rails server/console/etc. This commit takes option 2, drop haml_lint since it's a dev dependency that can easily be added in bundler.d directory for any developers who want it.
…id_haml_erubis_warning Drop haml_lint to avoid deprecated haml constant
…mpiled Assume test assets are precompiled
It was determined that using a `before(:all)` to setup the local "fake" git repo might have file system contamination issues when trying to run parallel specs. Since this operation is fairly quick, moving it to a `before` with the rest of the work there has little noticeable performance issues by adding the FileIO calls for each spec. Also, in addition to the constants being turned into `let` calls, they also use `Dir.mktmpdir` to make sure there is a unique `clone_dir` for each spec.
Drop scss_lint as ruby-sass is deprecated
…rugged-for-parallel-specs [configuration_script_source_spec] Avoid before(:all) with filesystem calls
for reports and the report data (right hand of screen) allow the use of inline sql views (aka the "turbo button") when there are virtual attributes in the primary query, they get run for every row (even those not brought back) this option allows rbac to optimize the query in these cases to run the virtual attributes in an outer query, so they only get run for the few rows brought back
virtual attributes should no contain aliases It should contain a grouping (parens on the outside) This alias was added due to a bug in virtual attributes that did not properly quote alias values. Now that that has been fixed, the alias can be removed.
default to using inline sql views for reports
Caused issues in the UI test suite. This is the quick fix to get that green again.
…-provision-reports Disable "turbo button" on Provisioning reports
Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
tadeboro
pushed a commit
that referenced
this pull request
Jun 26, 2019
…erer-filesystem-git-fixes [EmbeddedAnsible::ConfigurationScriptSource] stub REPO_DIR (fix attempt #3)
miha-plesko
pushed a commit
that referenced
this pull request
Sep 2, 2019
Instead of reading "stdout" for the `ansible-runner` invocation using
the `artifacts/result/stdout`, use the json file from the
`artifacts/result/job_events/` directory instead since they are more
consistently correct (it seems).
There is some additional sorting and data massaging that is also done to
make sure things are correct (and have been documented in the code
directly), but it is mostly:
- looping through the .json files in the directory
- sorting them to the proper numerical order
- Reading the files and appending them to a string
- Ensuring a new lines exist for each file read
This makes it so that each file's content is on it's own line in the
same expected format as the `artifacts/result/stdout` that was used
previously.
In addition to the code changes, the specs need to be updated to adapt
to the new files required to be generated. That said, the number of
expectations has decreased as a result since testing the "bad case" is
no longer necessary.
I am pretty sure the downsides to this approach are limited, and the
previous commit should make it so that any non-Hash values potentially
generated from this new code will be a non-issue. However, we do stop
using what is most likely the preferred file going forward.
As far as speed, there is a bit of a speed up using this approach, so again
using the following script to test:
5.times do
puts Benchmark.measure do
500.times do
Ansible::Runner::Response.new(:base_dir => Dir.pwd)
.parsed_stdout
.map {|l| l['stdout']}
.join("\n")
end
end
end
For the previous commit, the results were:
$ ruby benchmark_script.rb
2.821104 0.075949 2.897053 ( 2.937712)
2.864495 0.060510 2.925005 ( 2.970379)
2.921303 0.058061 2.979364 ( 3.016452)
3.191436 0.036860 3.228296 ( 3.285050)
3.079974 0.065541 3.145515 ( 3.225359)
And with these changes:
$ ruby benchmark_script.rb
1.497543 0.097888 1.595431 ( 1.616944)
1.524888 0.059816 1.584704 ( 1.610376)
1.504166 0.070907 1.575073 ( 1.599539)
1.550756 0.066972 1.617728 ( 1.641647)
1.525693 0.053345 1.579038 ( 1.601455)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PRs:
BIOS Update Request is created via API like this (just fyi, because React performs it on your behalf):