Refactor build / sign managers #361
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qbarrand The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
✅ Deploy Preview for kubernetes-sigs-kmm ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
k8s-ci-robot
added
the
size/XXL
Create a new internal/imgbuild package that brings a new Manager type
covering the needs of both features, as well as a JobMaker interface.
Implement that interface in internal/{build,sign} packages.
Consolidate ModuleLoaderData generation code into the internal/api
package.
Move utils functions into their own package and remove duplicate code.
qbarrand
force-pushed
the
refactor-build-sign
branch
from
b8b557b to
7c7da20
Compare
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
IMHO, this PR can be split into 3: imgbuild changes, ModuleLoaderData changes, utils changes. |
| @@ -0,0 +1,30 @@ | |||
| package imgbuild | |||
There was a problem hiding this comment.
can we rename this file to something else? maybe interfaces.go ? This way it will be more clear what are its contents
| JobTypeSign JobType = "sign" | ||
| ) | ||
|
|
||
| type BaseJobManager struct { |
There was a problem hiding this comment.
why do we need this struct public? It is used only in this package, no?
| } | ||
|
|
||
| func (bjm *BaseJobManager) Sync(ctx context.Context, mld *api.ModuleLoaderData, pushImage bool, owner metav1.Object) (Status, error) { | ||
| logger := log.FromContext(ctx).WithValues("type", bjm.jobType) |
There was a problem hiding this comment.
lets add at least Name from mld to the WithValues, so that we can find in the log which module are we talking about here
|
Extracted |
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signing Jobs can be created in any namespace, while the ConfigMaps containing the OpenShift CA certificates from the bundle are only present in the operator's namespace. In each namespace containing a Module or targeted by a ManagedClusterModule, create two ConfigMaps for each CA certificate type. Those ConfigMaps are populated by other OpenShift controllers. When creating a signing Job, look for those ConfigMaps in the Job's namespace and mount them accordingly. Set the ConfigMaps owner references appropriately so that they are deleted when all Modules / ManagedClusterModules that might need it are deleted.
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
k8s-ci-robot
removed
the
lifecycle/stale
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
k8s-ci-robot
added
lifecycle/rotten
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
|
@k8s-triage-robot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Create a new
internal/imgbuildpackage that brings a newManagertype covering the needs of both features, as well as aJobMakerinterface.Implement that interface in
internal/{build,sign}packages.Consolidate
ModuleLoaderDatageneration code into theinternal/apipackage.Move
utilsfunctions into their own package and remove duplicate code.