Keystone: support for Service controller

[winiciusallan]

This pull request introduces support for the Keystone Service controller

Closes: #429

Reviewer notes:

• Because of this issue, I needed to handle with Type field in some reconcile operations.
• I was in doubt about the adoption implementation, so checking in other controllers, I decided to just make available all current list options (name and type).

[github-actions[bot]]

Failed to assess the semver bump. See logs for details.

[mandre]

I have found it very helpful, when working on controllers after we created the scaffolding tool, to have at least 2 separate commits:

• the first one that just runs the scaffolding tool and commits all the generated files. We should write the exact command we ran in the commit message for reproducibility.
• a second commit that fills the blank and effectively implements the specific parts of the controller, including the API changes and the tests.
• more commits as necessary.

This helps figure out quickly the specific parts of the controller and find potential bugs and missing bits more easily. If we agree on this patter, we may want to add it as a requirement in our developers docs.

[github-actions[bot]]

Failed to assess the semver bump. See logs for details.

[winiciusallan]

This helps figure out quickly the specific parts of the controller and find potential bugs and missing bits more easily. If we agree on this patter, we may want to add it as a requirement in our developers docs.

As discussed in #567, we have some gaps to fill in the developer documentation. This may be a good addition.

I've reverted some stuff and pushed the commits. I will just need to make another commit to resolve merge conflicts.

[github-actions[bot]]

Failed to assess the semver bump. See logs for details.

[github-actions[bot]]

Failed to assess the semver bump. See logs for details.

[github-actions[bot]]

Failed to assess the semver bump. See logs for details.

[github-actions[bot]]

Failed to assess the semver bump. See logs for details.

[winiciusallan]

@mandre do you have a clue why make-generate is failing? I have run locally a couple of times and it seems ok. Replacing the diff text manually makes sense?

also, I already fixed the lint. I'm waiting for your response to the above question to commit once. 😄

[mandre]

@mandre do you have a clue why make-generate is failing? I have run locally a couple of times and it seems ok. Replacing the diff text manually makes sense?

I believe it's because of https://github.com/k-orc/openstack-resource-controller/pull/565, you'll need to rebase your change.

[winiciusallan]

This seems very close to a merging state. Really nice work! I've noticed you made iterative commits, where you added new commits to address review comments. That's OK during the review phase as it makes each change stand out, but we should rebase and squash the commits to keep clean history when we're happy with the PR status. Would you mind doing it before we merge your PR?

Thanks! That's ok for me!

For the sake of clarity, do you want me to keep only commits with changes after running the scaffolding-tool and the implementation itself?

Before merging, I would like to highlight something I noticed yesterday. I don't implement support for filling the Extra field, so if a user wants to add an email, for example, it won't be possible. Actually, I don't see many use cases for free-forms extra fields, but I've seen in gophercloud test files this such case.

[mandre]

This seems very close to a merging state. Really nice work! I've noticed you made iterative commits, where you added new commits to address review comments. That's OK during the review phase as it makes each change stand out, but we should rebase and squash the commits to keep clean history when we're happy with the PR status. Would you mind doing it before we merge your PR?

Thanks! That's ok for me!

For the sake of clarity, do you want me to keep only commits with changes after running the scaffolding-tool and the implementation itself?

The scaffolding should be its own commit (just running the command that is documented in the commit message + make generate), then all the necessary commits to implement the controller. Typically, you'd end up with 2 commits: the scaffolding and the implementation of the controller + tests, but it's very possible that you'd have more commits if for example you need to do refactoring or have other controllers make use of your new controllers. We want to split the commits in logical changes that make sense separately (I'll leave that up to you how you want to logically split your commits). Take a look at how @eshulman2 did on https://github.com/k-orc/openstack-resource-controller/pull/568, this is a very good example.

Before merging, I would like to highlight something I noticed yesterday. I don't implement support for filling the Extra field, so if a user wants to add an email, for example, it won't be possible. Actually, I don't see many use cases for free-forms extra fields, but I've seen in gophercloud test files this such case.

That's fine, we don't want to expose Extra from the ORC API because:

• it's undocumented and could go away at any time
• we don't need it

[winiciusallan]

Hi @mandre! Thanks for the clear explanation. I've decided to keep only two commits (one with scaffolding tool generated code, and another with controller implementation itself).

You may take a last look, but I believe it is now ready to go.