-
Notifications
You must be signed in to change notification settings - Fork 469
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
Move steps for migrating code to csi-operator to ocp enhancements #1644
Changes from 1 commit
1905962
d0883f9
e7d5e02
4e0d5bd
75ef547
9660e8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -469,6 +469,68 @@ N/A | |
|
||
Same as today. | ||
|
||
## Process of moving operators to csi-operator monorepo | ||
|
||
We have come with following flow for moving operators from their own repository into csi-operator mono repo. | ||
|
||
### Avoid .gitignore related footguns | ||
|
||
Remove any .gitignore entries in the source repository that would match a directory / file that we need. For example, `azure-disk-csi-driver-operator` in .gitignore matched `cmd/azure-disk-csi-driver-operator` directory that we really need not to be ignored. See https://github.com/openshift/csi-operator/pull/110, where we had to fix after merge to csi-operator. | ||
|
||
### Move existing code into csi-operator repository | ||
|
||
Using git-subtree move your existing operator code to https://github.com/openshift/csi-operator/tree/master/legacy | ||
|
||
``` | ||
git subtree add --prefix legacy/azure-disk-csi-driver-operator https://github.com/openshift/azure-disk-csi-driver-operator.git master --squash | ||
git subtree push --prefix legacy/azure-disk-csi-driver-operator https://github.com/openshift/azure-disk-csi-driver-operator.git master | ||
``` | ||
|
||
### Add Dockerfiles for building images from new location | ||
|
||
Place a `Dockerfile.<operator>` and `Dockerfile.<operator>.test` at top of csi-operator tree and make sure that you are able to build an image of the operator from csi-operator repository. | ||
|
||
### Update openshift/release to build image from new location | ||
Make a PR to openshift/release repository to build the operator from csi-operator. For example - https://github.com/openshift/release/pull/46233. | ||
|
||
1. Update also `storage-conf-csi-<operator>-commands.sh`, the test manifest will be at a different location. | ||
2. Make sure that rehearse jobs for both older versions of operator and newer versions of operator pass. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a better way to search for rehearsal job names other than looking at jobs in PRs and history? Maybe we could expand on this step a bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think when you do |
||
|
||
### Change ocp-build-data repository to ship image from new location | ||
|
||
Make a PR to [ocp-build-data](https://github.com/openshift-eng/ocp-build-data) repository to change location of the image etc - https://github.com/openshift-eng/ocp-build-data/pull/4148 | ||
|
||
1. Notice the `cachito` line in the PR - we need to build with the vendor from legacy/ directory. | ||
|
||
2. Ask ART for a scratch build. Make sure you can install a cluster with that build. | ||
|
||
``` | ||
oc adm release new \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not applicable to OLM based operators - can we have a note or pointer here on testing the scratch build with OLM? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added some wording for this. |
||
--from-release=registry.ci.openshift.org/ocp/release:4.15.0-0.nightly.XYZ \ | ||
azure-disk-csi-driver-operator=<the scratch build> \ | ||
--to-image=quay.io/jsafrane/scratch:release1 \ | ||
--name=4.15.0-0.nightly.jsafrane.1 | ||
|
||
oc adm release extract --command openshift-install quay.io/jsafrane/scratch:release1 | ||
``` | ||
|
||
### Co-ordinating merges in ocp-build-data and release repository | ||
|
||
Both PRs in openshift/release and ocp-build-data must be merged +/- at the same time. There is a robot that syncs some data from ocp-build-data to openshift/release and actually breaks things when these two repos use different source repository to build images. | ||
|
||
### Enjoy the build from csi-operator repository | ||
|
||
After aforementioned changes, your new operator should be able to be built from csi-operator repo and everything should work. | ||
|
||
### Moving operator to new structure in csi-operator | ||
|
||
So in previous section we merely copied existing code from operator’s own repository into `csi-operator` repository. We did not change anything. | ||
|
||
But once your operator has been changed to conform to new code in csi-operator repo, You need to perform following additional steps: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the notion of "to conform to new code" deserves more words/details. I'd highlight the following topics:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not want to tie this enhancement to code and asset structure. What if that changes? An enhancement is not the right place to document implementation details TBH (at least not in great detail). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, agreed, let's keep it vague. |
||
|
||
1. Make sure that `Dockerfile.<operator>` at top of the `csi-operator` tree refers to new location of code and not older `legacy/<operator>` location.See example of existing Dockerfiles. | ||
gnufied marked this conversation as resolved.
Show resolved
Hide resolved
|
||
2. After your changes to `csi-operator` are merged, you should remove the old location from cachito - https://github.com/openshift-eng/ocp-build-data/pull/4219 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would explicitly mention that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
|
||
## Implementation History | ||
|
||
Major milestones in the life cycle of a proposal should be tracked in `Implementation | ||
|
@@ -548,4 +610,4 @@ Advantages: | |
|
||
## Infrastructure Needed [optional] | ||
|
||
N/A (other than the usual CI + QE) | ||
N/A (other than the usual CI + QE) |
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.
We consolidated the various
Dockerfile.<operator>.test
files into a singleDockerfile.test
, there generally shouldn't be any need to add a new one unless some operator is doing more thanFROM src
.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.
fixed.