Skip to content

Conversation

@cgwalters
Copy link
Member

We need a development workflow that keeps the CVO active so we
can test upgrades.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 25, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 25, 2019
@cgwalters
Copy link
Member Author

Still working on this but uploading images over 📱 tethering is very slow...going to need to wait till I have faster internets.

(Or...investigate a workflow building images on the cluster itself, but that goes into a bigger 🐇 hole)

@cgwalters
Copy link
Member Author

If we go this route it will obsolete #399 I think.

@cgwalters
Copy link
Member Author

cgwalters commented Feb 26, 2019

OK this is going along, but I've hit a hurdle:

+ oc adm release new --from-release registry.svc.ci.openshift.org/openshift/origin-release@sha256:13107213ca816f906b634a930ed8a4a416d09ddc11e4e9e4d8794f5e7551989b --to-image=openshift-machine-config-operator/dev
release:latest machine-config-operator=image-registry-openshift-image-registry.apps.osiris.verbum.local/openshift-machine-config-operator/machine-config-operator:latest machine-config-controller=image-registry-o
penshift-image-registry.apps.osiris.verbum.local/openshift-machine-config-operator/machine-config-controller:latest machine-config-server=image-registry-openshift-image-registry.apps.osiris.verbum.local/openshift-machine-config-operator/machine-config-server:latest machine-config-daemon=image-registry-openshift-image-registry.apps.osiris.verbum.local/openshift-machine-config-operator/machine-config-daemon:latest
info: Found 73 images in release                                                                                                   
info: Manifests will be extracted to /tmp/release-image-0.0.1-2019-02-26-155342680362880                                           
...
error: unable to connect to image repository image-registry-openshift-image-registry.apps.osiris.verbum.local/openshift-machine-config-operator/machine-config-controller:latest: Get https://image-registry-openshift-image-registry.apps.osiris.verbum.local/v2/: x509: certificate signed by unknown authority

I think this is because the oc image mirror code that ends up being invoked by oc adm release new doesn't know to look at my KUBECONFIG to trust the certificates there. Which is understandable.

I guess this is specific to libvirt today. Hmm. Actually...is it possible to provide a custom CA root to the installer? If so I could do that once and use it for every cluster. /me looks

@cgwalters
Copy link
Member Author

I guess this is specific to libvirt today.

No, it's not; I had forgotten that today in 4.0 clusters the router uses a self-signed cert by default.

I am working on this patch for origin:

diff --git a/pkg/oc/cli/admin/release/mirror.go b/pkg/oc/cli/admin/release/mirror.go
index 08a4c212ac..6d5eea9f82 100644
--- a/pkg/oc/cli/admin/release/mirror.go
+++ b/pkg/oc/cli/admin/release/mirror.go
@@ -68,6 +68,7 @@ func NewMirror(f kcmdutil.Factory, parentName string, streams genericclioptions.
 	flags.StringVar(&o.From, "from", o.From, "Image containing the release payload.")
 	flags.StringVar(&o.To, "to", o.To, "An image repository to push to.")
 	flags.BoolVar(&o.DryRun, "dry-run", o.DryRun, "Display information about the mirror without actually executing it.")
+	flags.BoolVar(&o.Insecure, "insecure", o.Insecure, "Allow push and pull operations to registries to be made over HTTP")
 
 	flags.BoolVar(&o.SkipRelease, "skip-release-image", o.SkipRelease, "Do not push the release image.")
 	flags.StringVar(&o.ToRelease, "to-release-image", o.ToRelease, "Specify an alternate locations for the release image instead as tag 'release' in --to")
@@ -85,6 +86,7 @@ type MirrorOptions struct {
 
 	RegistryConfig string
 	DryRun         bool
+	Insecure       bool
 
 	ImageStream *imageapi.ImageStream
 	TargetFn    func(component string) imagereference.DockerImageReference
@@ -237,6 +239,7 @@ func (o *MirrorOptions) Run() error {
 	opts.RegistryConfig = o.RegistryConfig
 	opts.Mappings = mappings
 	opts.DryRun = o.DryRun
+	opts.Insecure = o.Insecure
 	opts.ManifestUpdateCallback = func(registry string, manifests map[digest.Digest]digest.Digest) error {
 		lock.Lock()
 		defer lock.Unlock()
diff --git a/pkg/oc/cli/admin/release/new.go b/pkg/oc/cli/admin/release/new.go
index 1393e97493..73c385def5 100644
--- a/pkg/oc/cli/admin/release/new.go
+++ b/pkg/oc/cli/admin/release/new.go
@@ -123,6 +123,7 @@ func NewRelease(f kcmdutil.Factory, parentName string, streams genericclioptions
 
 	// destination
 	flags.BoolVar(&o.DryRun, "dry-run", o.DryRun, "Skips changes to external registries via mirroring or pushing images.")
+	flags.BoolVar(&o.Insecure, "insecure", o.Insecure, "Allow push and pull operations to registries to be made over HTTP")
 	flags.StringVar(&o.Mirror, "mirror", o.Mirror, "Mirror the contents of the release to this repository.")
 	flags.StringVar(&o.ToDir, "to-dir", o.ToDir, "Output the release manifests to a directory instead of creating an image.")
 	flags.StringVar(&o.ToFile, "to-file", o.ToFile, "Output the release to a tar file instead of creating an image.")
@@ -165,6 +166,7 @@ type NewOptions struct {
 	PreviousVersions []string
 
 	DryRun bool
+	Insecure bool
 
 	ToFile         string
 	ToDir          string
@@ -859,6 +861,7 @@ func (o *NewOptions) mirrorImages(is *imageapi.ImageStream, payload *Payload) er
 	opts.To = o.Mirror
 	opts.SkipRelease = true
 	opts.RegistryConfig = o.RegistryConfig
+	opts.Insecure = o.Insecure
 
 	if err := opts.Run(); err != nil {
 		return err

But no luck so far, I think I need to fix something in registryclient too.

@kikisdeliveryservice
Copy link
Contributor

@cgwalters lmk when this is ready to test. Happy to help!

@cgwalters
Copy link
Member Author

Hm, now I'm getting:

$ oc -n openshift-ingress port-forward svc/router-internal-default 443
error: error upgrading connection: error dialing backend: remote error: tls: internal error

@cgwalters
Copy link
Member Author

/test e2e-aws-op

@cgwalters
Copy link
Member Author

/retest

@cgwalters
Copy link
Member Author

error: error upgrading connection: error dialing backend: remote error: tls: internal error

Ah, awesome this is a workaround:
openshift/cluster-api-provider-libvirt#128 (comment)

We need a development workflow that keeps the CVO active so we
can test upgrades.
@cgwalters
Copy link
Member Author

cgwalters commented Mar 19, 2019

OK so...still playing with this. One struggle I had was building images inside vs outside the cluster; the payload needs to use the in-cluster registry pull spec at least with libvirt.

So I switched to doing oc adm release new inside a Job but then I ran into the fact the cli image doesn't trust the cluster's CA (same issue external):

$ oc -n openshift-cluster-version logs jobs/mco-build-update-image
error: unable to connect to image repository image-registry.openshift-image-registry.svc:5000/openshift-machine-config-operator/mcodev:latest: Get https://image-registry.openshift-image-registry.svc:5000/v2/: x509: certificate signed by unknown authority

@cgwalters
Copy link
Member Author

I think we should land #682 first and then try to factor out the common parts of this PR and that code. And then decide whether we always test via the CVO, i.e. we drop make deploy-$x, or if we have a faster path that skips the CVO.

@cgwalters
Copy link
Member Author

May try to revisit this...

@openshift-ci-robot
Copy link
Contributor

@cgwalters: PR needs rebase.

Details

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.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2019
@openshift-ci-robot
Copy link
Contributor

@cgwalters: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-op d3fc7f9 link /test e2e-aws-op
ci/prow/e2e-aws d3fc7f9 link /test e2e-aws
ci/prow/e2e-aws-disruptive d3fc7f9 link /test e2e-aws-disruptive
ci/prow/e2e-gcp-op d3fc7f9 link /test e2e-gcp-op
ci/prow/e2e-aws-upgrade d3fc7f9 link /test e2e-aws-upgrade
ci/prow/e2e-gcp-upgrade d3fc7f9 link /test e2e-gcp-upgrade
ci/prow/e2e-vsphere d3fc7f9 link /test e2e-vsphere
ci/prow/build-rpms-from-tar d3fc7f9 link /test build-rpms-from-tar

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

@kikisdeliveryservice
Copy link
Contributor

In an effort to clean up the MCO repo, closing old open PRs with no recent activity.

Feel free to reopen.

@kikisdeliveryservice kikisdeliveryservice added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants