[WIP] Add new README file for Console Operator #1006
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Leo6Leo The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ath configuration
…tion and management steps
There was a problem hiding this comment.
Overall, these are pretty radical changes, I think we should keep things simple and straightforward which is what the current README.md tried to do.
Maybe keep the structure that is in place now in README.md, fix some outdated info, improve the language and maybe link some sort of script in the Tips section, if we think thats needed.
deploy-custom-operator.sh
Outdated
|
|
||
| set -e | ||
|
|
||
| echo "🚀 Console Operator Custom Deployment Script" |
There was a problem hiding this comment.
I would remove all the emojis, they are out of place here.
There was a problem hiding this comment.
Yes, I agree, as this is not upstream project and it should be more professional.
deploy-custom-operator.sh
Outdated
| # Get cluster ID dynamically | ||
| echo "📋 Getting cluster ID..." | ||
| CLUSTER_ID=$(oc get clusterversion version -o jsonpath='{.spec.clusterID}') | ||
| if [ -z "$CLUSTER_ID" ]; then | ||
| echo "❌ Error: Could not retrieve cluster ID" | ||
| exit 1 | ||
| fi | ||
| echo "✅ Cluster ID: $CLUSTER_ID" | ||
|
|
||
| # Create temporary CVO unmanage configuration | ||
| echo "📝 Creating CVO unmanage configuration..." | ||
| TEMP_CVO_CONFIG=$(mktemp) | ||
| cat > "$TEMP_CVO_CONFIG" << EOF | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: ClusterVersion | ||
| metadata: | ||
| name: version | ||
| spec: | ||
| clusterID: $CLUSTER_ID | ||
| overrides: | ||
| - kind: Deployment | ||
| name: console-operator | ||
| namespace: openshift-console-operator | ||
| unmanaged: true | ||
| group: apps | ||
| - kind: ClusterRole | ||
| name: console-operator | ||
| namespace: "" | ||
| unmanaged: true | ||
| group: rbac.authorization.k8s.io | ||
| EOF | ||
|
|
||
| # Step 1: Disable CVO management of console operator | ||
| echo "" | ||
| echo "📋 Step 1: Disabling CVO management of console operator..." | ||
| oc apply -f "$TEMP_CVO_CONFIG" | ||
| echo "✅ CVO management disabled" |
There was a problem hiding this comment.
This is over-engineered. We have these config files in examples folder.
There was a problem hiding this comment.
I overlooked it, thanks for catching it!
newreadme.md
Outdated
| - Go 1.23.0 or later | ||
| - Docker | ||
| - OpenShift CLI (`oc`) | ||
| - Access to an OpenShift cluster |
There was a problem hiding this comment.
I know that in the original README we mentioned GVM as well, I'm not using it myself, so can't say if its still usable for CO work. Should we keep that?
There was a problem hiding this comment.
Since Go 1.21, we can use the go command to manage and switch between different Go SDK versions. You can specify a particular Go version for the project in the go.mod file, and the Go toolchain will automatically download and use that version. Therefore the third party library like gvm is not needed anymore. That's my understanding on this.
Reference can be found here
newreadme.md
Outdated
| 3. **Deploy using the custom operator script**: | ||
| ```bash | ||
| # Make the script executable | ||
| chmod +x deploy-custom-operator.sh | ||
|
|
||
| # Run the deployment script | ||
| ./deploy-custom-operator.sh | ||
| ``` | ||
|
|
There was a problem hiding this comment.
I don't think we should solely rely on people to run a script to deploy and run the CO. The script might be usable as a supplemental tool, for making sure everything is configured right in an automatized way. But for example in the real developer scenario all you need to do is to rebuild the CO, push the new image to the repository and redeploy the CO pod. We should keep the useful commands and information in the README as we had previously.
There was a problem hiding this comment.
Agree, I have make the edit to the readme to keep both options there.
Thanks for the feedback Marek!! I totally agree with your goal of keeping the README.md simple and straightforward. Based on my recent onboarding experience, the current README.md, while it is very detailed, but it can feel a bit verbose and includes some outdated information about manual Go processes. For someone fresh to the project, this can sometimes make getting started a bit slower than it needs to be. The updated version aims to streamline that process with a more structured layout and an automated script to reduce setup friction. I see it less as a radical change, and more as a reorganization to improve clarity and relevance for today’s workflow. I understand your concern about "radical changes." I think that we don't want to disregard the existing content, but rather to present it in a more intuitive and modern layout. I believe a fresh structure will make it easier to navigate for everyone, while still containing the necessary information. What do you think about us moving forward with outlining a new structure and then filling in the content? Let me know what you think! |
…anual CVO management steps, and include helpful debugging commands
Leo6Leo
changed the title
I appreciate the effort you put into this, we can definitely open the discussions about the changes of the structure. Before we do that would you mind changing the name of the newreadme.md to README.md, so that we could see the diff between the old version and new version ? |
Background
The existing README was outdated and reflected development practices from an earlier era of Go development and OpenShift tooling. It contained complex GOPATH setup instructions, outdated Go version requirements, and verbose cluster creation steps that are no longer relevant to modern development workflows. Making this PR to gradually improve the documentation and ensure the developer experience is up-to-date.
References & Accredition
This PR has incooperated the changes that @Mylanos has done in #982 .