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.

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

Upgrade the operator to use Operator SDK v1.33.0 #182

Merged
merged 19 commits into from
Jan 25, 2024

Conversation

edif2008
Copy link
Member

@edif2008 edif2008 commented Jan 5, 2024

Summary

This PR migrates the operator from version Operator SDK v1.29.0 to v1.33.0.

Previously we were using go/v4-alpha. With the latest changes, this went to stable (i.e. go/v4). This brought a couple of key changes:

  • Project restructure
  • Use Kustomize 4.x instead of 3.x

In addition, we've updated dependencies and adjusted the code based on the breaking changes present in the sigs.k8s.io/controller-runtime package from v0.14.5 to v0.16.3.

Resolves: #180

Details

The changes present in this PR can be divided into four parts:

  • Project restructure
    • Move files in controllers to internal/controller directory
    • Move main.go from project's root directory to cmd
  • Config scaffolding -> Most of the changes here are made to use Kustomize 4.x instead of Kustomize 3.x. These don't affect the image users use to deploy the operator in their infrastructure, as well as their configurations.
  • Go updates
    • Update package dependencies
    • Update Go version to 1.21
  • Code changes
    • There is only one code change that was made to achieve the same thing with the latest functions and structures available in the sigs.k8s.io/controller-runtime package. From a customer's experience, this shouldn't be a breaking change.

⚠️ With the latest changes in the Kubernetes, it seems that the Connect images require root privileges, which is not allowed by the Connect-related yaml files due to having securityContext -> runAsNonRoot: true. This also affects previous versions of the operator. It looks like we haven't noticed this concern (or my environment is weird and it shouldn't be an issue).

How to test

Requirements

Testing steps

  1. Checkout this branch:
    git checkout feat/upgrade-operator-sdk-1.33.0
  2. Enter the cluster's environment and build the operator's image inside with make docker-build. The steps below show how to do this with minikube:
    eval $(minikube docker-env) && make docker-build
  3. Adjust the config/manager/manager.yaml with the following:
    • Add imagePullPolicy: Never so that it uses the image of the operator you've just built:
      image: 1password/onepassword-operator:latest
      imagePullPolicy: Never
    • change the value of the WATCH_NAMESPACE environment variable with the name of the namespace you want to use:
      - name: WATCH_NAMESPACE
         value: "<your-namespace>"
    • Deploy Connect alongside the operator and enable auto-restarting:
      - name: AUTO_RESTART
         value: "true"
      - name: MANAGE_CONNECT
         value: "true"
  4. Create a new namespace in which you will deploy the operator:
    kubectl create namespace <your-namespace>
  5. To simplify the next steps, change the Kubernetes configuration to set the newly created namespace as the default one. Below is an example of doing that for minikube:
    kubectl config set-context minikube --namespace=<your-namespace>
  6. Create the op-credentials and the onepassword-token Kubernetes secrets in the namespace:
    # Create 'op-credentials' Kubernetes secret
    cat 1password-credentials.json | base64 | \
        tr '/+' '_-' | tr -d '=' | tr -d '\n' > op-session && \
        kubectl create secret generic op-credentials --from-file=op-session
    
    # Create 'onepassword-token' Kubernetes secret
    kubectl create secret generic connect-token --from-literal=token="<your-Connect-token>"
  7. Deploy the operator in that namespace:
    make deploy
  8. Start playing around with the operator. No changes should be present in the operator's behavior.

Check for any missed changes in the migration

These steps require to install the operator-sdk.

The steps below are adapted based on the Migration steps from go/v3 to go/v4 with the key change of creating the project and API using the operator-sdk command instead of kubebuilder.

  1. Create a separate directory in which you will create the scaffolding of the operator using version v1.33.0:
    mkdir tst-migrated-operator/onepassword-operator && cd tst-migrated-operator/onepassword-operator
  2. Initialize the operator:
    operator-sdk init --plugins go/v4 --domain onepassword.com  --repo github.com/1Password/onepassword-operator --license=none
  3. Create the OnePasswordItem API:
    operator-sdk create api --version v1 --kind OnePasswordItem --resource --controller
  4. Copy from this branch the following:
    • cmd/main.go
    • api/v1/onepassworditem_types.go
    • internal/controller directory
    • pkg directory
  5. Build the operator:
    make all
  6. Compare this branch against the directory in which you created the operator. The following differences that may be present can be ignored:
    • Newer Go package dependencies
    • Newer tools used in Makefile (e.g. ENVTEST_K8S_VERSION, KUSTOMIZE_VERSION, CONTROLLER_TOOLS_VERSION)
    • Dockerfile adjustments to copy additional files needed, as well as adding our custom flags for building the operator.

Reference

Based on the go/v4 project structure, the following changed:
- Pakcage `controllers` is now named `controller`
- Package `controller` now lives inside new `internal` directory
Based on the new go/v4 project structure, `main.go` now lives in the `cmd` directory.
Update the dependencies based on the versions obtained by creating a new operator project using `kubebuilder init --domain onepassword.com --plugins=go/v4`.

This is based on the migration steps provided to go from go/v3 to go/v4 (https://book.kubebuilder.io/migration/migration_guide_gov3_to_gov4)
sigs.k8s.io/controller-runtime package had breaking changes from v0.14.5 to v0.16.3. This commit brings the changes needed to achieve the same things using the new functionality avaialble.
Since `main.go` is now in `cmd` directory, the paths to the files for deploying Connect have to be adjusted based on the new location `main.go` is executed from.
These changes are made based on the new project structure and scaffolding obtained when using the new go/v4 project structure.

These were done based on the migration steps mentioned when migrating to go/v4 (https://book.kubebuilder.io/migration/migration_guide_gov3_to_gov4).
These updates are made based on the Kustomize v4 syntax.

This is part of the upgrate to go/v4 (https://book.kubebuilder.io/migration/migration_guide_gov3_to_gov4)
Now the version in the Makefile matches the version of the operator
@edif2008 edif2008 changed the title Draft: Upgrade the operator to use Operator SDK v1.33.0 Upgrade the operator to use Operator SDK v1.33.0 Jan 5, 2024
cmd/main.go Show resolved Hide resolved
It seems that the +build tag is no longer needed based on the latest generated scaffolding, therefore it's removed.
@volodymyrZotov volodymyrZotov self-assigned this Jan 17, 2024
Some customization in Makefile was lost during the migration process. Specifically, the namespace customization for `make deploy` command.

Also, we push changes to kustomization.yaml for making the deploy process smoother.
It seems that with the latest changes to Kubernetes and Kustomize, we need to add additional RBAC to the service account used so that it can properly access the `leases` resource.
Dockerfile Outdated Show resolved Hide resolved
@volodymyrZotov
Copy link
Collaborator

Functional review: ✅

Confirm that the operator works fine after migration.


Code review: ✅

Most of the changes here are because of updating dependencies in vendor folder, but the rest looks good to me.
Approve this PR, but before merging, please resolve the last comment on Dockerfile

Dockerfile had a step for caching dependencies (go mod download). However, this is already done by the vendor directory, which we include. Therefore, this step can be removed to make the image build time faster.
@edif2008 edif2008 merged commit f72e524 into main Jan 25, 2024
2 checks passed
@edif2008 edif2008 deleted the feat/upgrade-operator-sdk-1.33.0 branch January 25, 2024 13:21
@edif2008 edif2008 self-assigned this Jan 25, 2024
@ScubaDrew
Copy link

THANK YOU 1P TEAM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

panic: runtime error: invalid memory address or nil pointer dereference
3 participants