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.

Sign up for GitHub

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

Jump to bottom

doc: migration guide for Ansible and quickstart fixes #3571

Merged
merged 4 commits into from
Aug 5, 2020
Merged

doc: migration guide for Ansible and quickstart fixes #3571

merged 4 commits into from
Aug 5, 2020

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Jul 27, 2020 •
edited
Loading

Description of the change:

Motivation for the change:
Partial closes: #3542 #3590
#3327

Follow-up of this one will be : operator-framework/ansible-operator-plugins#26 (to update the molecule docs)

PS.: PR to migrate the sample: operator-framework/operator-sdk-samples#138

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2020
@camilamacedo86 camilamacedo86 changed the title WIP: doc: migration guide for Ansbile doc: migration guide for Ansbile Jul 27, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2020
@camilamacedo86 camilamacedo86 changed the title doc: migration guide for Ansbile wip: doc: migration guide for Ansbile Jul 27, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2020
@camilamacedo86 camilamacedo86 changed the title wip: doc: migration guide for Ansbile doc: migration guide for Ansbile Jul 27, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2020
@camilamacedo86 camilamacedo86 changed the title doc: migration guide for Ansbile wip:doc: migration guide for Ansbile Jul 27, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2020
@camilamacedo86 camilamacedo86 force-pushed the migration-ansible branch 10 times, most recently from 0379e9c to 0b85e9f Compare July 29, 2020 11:29
@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Jul 29, 2020 •
edited
Loading

Hi @joelanford @fabianvf @asmacdo,

I think it is good to be merged to allow we have the docs and work easier with that but we will still need to do a follow up to improve the molecule section and after the migration. Could we work in this way?

@camilamacedo86 camilamacedo86 changed the title wip:doc: migration guide for Ansbile doc: migration guide for Ansbile Jul 29, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 29, 2020
@camilamacedo86 camilamacedo86 force-pushed the migration-ansible branch 2 times, most recently from f223122 to ae2692d Compare July 29, 2020 11:57
asmacdo
asmacdo reviewed Jul 30, 2020
View reviewed changes
website/content/en/docs/building-operators/ansible/migration.md Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/tutorial.md Outdated Show resolved Hide resolved
@camilamacedo86 camilamacedo86 force-pushed the migration-ansible branch 2 times, most recently from dd05c66 to bfd1fb8 Compare July 30, 2020 18:58
asmacdo
asmacdo approved these changes Jul 30, 2020
View reviewed changes
Copy link
Member

@asmacdo asmacdo left a comment

Choose a reason for hiding this comment

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

Some nits, no need for re-review

website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/migration.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/quickstart.md Outdated Show resolved Hide resolved
website/content/en/docs/building-operators/ansible/quickstart.md Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 5, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 5, 2020
fabianvf
fabianvf reviewed Aug 5, 2020
View reviewed changes
Copy link
Member

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

Couple small issues, mostly looks good though

website/content/en/docs/building-operators/ansible/migration.md Outdated

And now:

- Use the var `${MOLECULE_PROJECT_DIRECTORY}` instead of `${MOLECULE_EPHEMERAL_DIRECTORY}`
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what these are referring to, I don't think they should change their usage of these variables

Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Aug 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I will remove this since we have a following up for the molecule.

website/content/en/docs/building-operators/ansible/migration.md Outdated
And now:

- Use the var `${MOLECULE_PROJECT_DIRECTORY}` instead of `${MOLECULE_EPHEMERAL_DIRECTORY}`
- Use the var `${KUBECONFIG:-"~/.kube/config"}` instead of `${MOLECULE_EPHEMERAL_DIRECTORY}/kubeconfig` to get the kubeconfig
Copy link
Member

Choose a reason for hiding this comment

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

This is only true for the default scenario (which runs against an existing cluster), the kind scenario still uses the ephemeral kubeconfig

Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Aug 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I will remove this since we have a following up for the molecule.
We can re-check it in a follow-up.

@joelanford joelanford mentioned this pull request Aug 5, 2020
Closed
92 tasks
jmrodri
jmrodri requested changes Aug 5, 2020
View reviewed changes
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

molecule is spelled wrong. And another nit.

website/content/en/docs/building-operators/ansible/migration.md Outdated
* RBAC manifests in `deploy` are now in `config/rbac/`

- `build/Dockerfile` is moved to `Dockerfile` in the project root directory
- The `molecue/` directory is now more aligned to Ansible and the new Layout
Copy link
Member

Choose a reason for hiding this comment

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

this should be molecule.

website/content/en/docs/building-operators/ansible/migration.md Outdated

## How to migrate

The easy migration path is to create a new project from the scratch and let the tool scaffold the new layout. Then, add your customizations and implementations. See below for an example.
Copy link
Member

Choose a reason for hiding this comment

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

project from scratch

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

@jmrodri jmrodri changed the title doc: migration guide for Ansbile and quickstart fixes doc: migration guide for Ansible and quickstart fixes Aug 5, 2020
@jmrodri jmrodri merged commit f04ad07 into operator-framework:master Aug 5, 2020
@camilamacedo86 camilamacedo86 deleted the migration-ansible branch August 5, 2020 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wkulhanek wkulhanek wkulhanek left review comments

@fabianvf fabianvf fabianvf left review comments

@asmacdo asmacdo asmacdo approved these changes

@jmrodri jmrodri jmrodri approved these changes

@joelanford joelanford Awaiting requested review from joelanford

@jmccormick2001 jmccormick2001 Awaiting requested review from jmccormick2001

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create migration guide from Legacy to new Layout for Ansible Projects
7 participants
@camilamacedo86 @jmrodri @joelanford @asmacdo @fabianvf @wkulhanek @openshift-ci-robot