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.
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
[Kogito-2195] Kogito examples with ansible automation #258
base: main
Are you sure you want to change the base?
[Kogito-2195] Kogito examples with ansible automation #258
Changes from 3 commits
1e07b99
654af19
f3971c0
4dee7f9
7c027cd
73d1f78
5eb0cda
48847f8
85b42af
a0cc19a
d78b179
b188a30
d157a97
8eb988a
2df8866
baba573
b77023d
31ac8dd
cd77312
cd487d7
2c80cf1
fa52112
be88c25
35f6e3a
da2fcb6
e76eb8e
260aa18
5489c7b
1fae857
78f4018
18c5e10
da06eff
bb7c906
4df8df3
f904f2c
0f04340
ebd0d10
87eb4ff
7762734
fc05cff
7138182
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide setup also for Fedora.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll create two sections in the doc since is all for fedora and only the lib virt installation is the first step on ubuntu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this section actually different from the
Fedora
one?They are really similar (the same?) from what I can see. Is it possible to create a single
Fedora/Ubuntu
section and just highlight different steps (if any).This can reduce maintenance cost of this doc :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously only one section plus the ubuntu lib virt installation was present but at the reviewer seems a document for ubuntu deploy and asked a fedora version, for this reason you see explicit configurations for both systems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree to have the instructions for
Ubuntu
andFedora
too but if they are the same just mention that the instructions are tested on both configuration instead of duplicate them :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this value hardcoded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because is used in the path and you discover the version only when the crc is downloaded and unpacked, or readed from the lates json info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've writed a version to use the latest kogito tag, but the 0.10.1 doesn't work because the cli and the operator aren't aligned (kogito 0.10.1 - kogito-cloud-operator 0.10.0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm I expect this could happen in the future too so is it possible to add a
kogito-operator
variable to support also this use case?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not use this value which can probably change on different machine etc. It would be much better to use "virt" or "virt_net" modules to obtain info about IP address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use Ansible DNF module instead https://docs.ansible.com/ansible/latest/modules/dnf_module.html#dnf-module
Something like this:
- name: Install Virtualization on Fedora
dnf:
name: "@Virtualization"
become: yes
when: ansible_os_family == 'RedHat'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok