Release 4.8.10 #1703

m1kola · 2021-08-26T15:57:42Z

Which issue this PR addresses:

Work Item №10405746.

What this PR does / why we need it:

Brings installer 4.8 into the RP.

Test plan for issue:

E2E run
Create a cluster locally

Is there any documentation that needs to be updated for this PR?

No. User facing communications are being handled separately.

m1kola

I recommend reviewing this commit-by-commit. I moved vendoring and generation of assets into separate commits.

I left few comments below which explain why we need some of ARO-RP changes.

m1kola · 2021-09-08T10:48:13Z

pkg/util/rhcos/rhcos.go

+// VHD fetches the URL of the public Azure blob containing the RHCOS image
+func VHD(ctx context.Context, arch types.Architecture) (string, error) {
+	archName := coreosarch.RpmArch(string(arch))
+	st, err := rhcospkg.FetchCoreOSBuild(ctx)
+	if err != nil {
+		return "", err
+	}
+	streamArch, err := st.GetArchitecture(archName)
+	if err != nil {
+		return "", err
+	}
+	ext := streamArch.RHELCoreOSExtensions
+	if ext == nil {
+		return "", fmt.Errorf("%s: No azure build found", st.FormatPrefix(archName))
+	}
+	azd := ext.AzureDisk
+	if azd == nil {
+		return "", fmt.Errorf("%s: No azure build found", st.FormatPrefix(archName))
+	}
+
+	return azd.URL, nil
+}


New code due to openshift/installer#4582

m1kola · 2021-09-08T10:49:44Z

pkg/util/rhcos/rhcos.go

+// Image returns an image object containing VM image SKU information.
+func Image(ctx context.Context) (*azuretypes.Image, error) {
+	osImage, err := VHD(ctx, types.ArchitectureAMD64)
+	if err != nil {
+		return nil, err
+	}
+
+	m := rxRHCOS.FindStringSubmatch(osImage)
+	if m == nil {
+		return nil, fmt.Errorf("couldn't match osImage %q", osImage)
+	}
+
+	return &azuretypes.Image{
+		Publisher: "azureopenshift",
+		Offer:     "aro4",
+		SKU:       "aro_" + m[2], // "aro_4x"
+		Version:   m[1],          // "4x.yy.2020zzzz"
+	}, nil
+}


This previously was called getRHCOSImage in pkg/cluster/generateconfig.go.

m1kola · 2021-09-08T10:53:08Z

hack/update-go-module-dependencies.sh

-go mod edit -replace github.com/metal3-io/cluster-api-provider-baremetal=$(go list -mod=mod -m github.com/openshift/cluster-api-provider-baremetal@release-4.7 | sed -e 's/ /@/')
-# We use release-4.8 here so we can use new version of sigs.k8s.io/controller-runtime sooner.
-go mod edit -replace github.com/metal3-io/baremetal-operator=$(go list -mod=mod -m github.com/openshift/baremetal-operator@release-4.8 | sed -e 's/ /@/')


These package are now pinned to a specific version (at leat for now). See in go.mod:

github.com/metal3-io/baremetal-operator => github.com/openshift/baremetal-operator v0.0.0-20210706141527-5240e42f012a // Use OpenShift fork github.com/metal3-io/baremetal-operator/apis => github.com/openshift/baremetal-operator/apis v0.0.0-20210706141527-5240e42f012a // Use OpenShift fork github.com/metal3-io/cluster-api-provider-baremetal => github.com/openshift/cluster-api-provider-baremetal v0.0.0-20210721192732-726d97e15db7 // Pin OpenShift fork

We pin them to specific commits and I think these commits are from master. It was huge pain to make these two work (and we don't even need them). So it doesn't really matter.

m1kola · 2021-09-08T10:58:05Z

pkg/operator/controllers/node/node_controller.go

@@ -107,7 +107,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 		GracePeriodSeconds:  -1,
 		IgnoreAllDaemonSets: true,
 		Timeout:             60 * time.Second,
-		DeleteLocalData:     true,
+		DeleteEmptyDirData:  true,


This is due to kubernetes/kubernetes#95076

m1kola · 2021-09-08T10:58:54Z

pkg/metrics/statsd/k8s/metrics.go

@@ -31,14 +32,14 @@ func NewResult(m metrics.Interface) kmetrics.ResultMetric {
 	}
 }

-func (t *tracer) Observe(verb string, url url.URL, latency time.Duration) {
+func (t *tracer) Observe(ctx context.Context, verb string, url url.URL, latency time.Duration) {


Interface change in k8s.io/client-go/tools/metrics. Same as below.

m1kola · 2021-09-08T12:48:48Z

pkg/util/dynamichelper/dynamichelper_test.go

@@ -81,6 +81,7 @@ func TestMerge(t *testing.T) {
 						"openshift.io/sa.scc.supplemental-groups": "groups",
 						"openshift.io/sa.scc.uid-range":           "uids",
 					},
+					Labels: map[string]string{"kubernetes.io/metadata.name": "test"},


NamespaceDefaultLabelName feature gate is enabled by default in Kubernetes 1.21: https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/

m1kola · 2021-09-08T13:02:47Z

pkg/cluster/generateconfig.go

@@ -142,6 +141,7 @@ func (m *manager) generateInstallConfig(ctx context.Context) (*installconfig.Ins
 						EncryptionAtHost: m.doc.OpenShiftCluster.Properties.MasterProfile.EncryptionAtHost == api.EncryptionAtHostEnabled,
 						OSDisk: azuretypes.OSDisk{
 							DiskEncryptionSetID: m.doc.OpenShiftCluster.Properties.MasterProfile.DiskEncryptionSetID,
+							DiskSizeGB:          1024,


openshift/installer#4666 breakes our code: we were relying on defaults set here and the fact that installer was mutating the isntallconfig object:

https://github.com/openshift/installer/blob/94125b21304a574ae5bc98039f8eb7f518293b83/pkg/asset/machines/master.go#L252-L253

But these defaults are not longer propogated to the installer config becuase this:

https://github.com/openshift/installer/blob/94125b21304a574ae5bc98039f8eb7f518293b83/pkg/asset/machines/master.go#L150

changed to this in 4.8:

https://github.com/openshift/installer/blob/c06e9b1562ccb76ac23163fb8db35f231ead6e97/pkg/asset/machines/master.go#L150

Fixed it by adding default value to our pkg/cluster/generateconfig.go.

Updates the codebase to allow vendoring of OCP 4.8

m1kola · 2021-09-08T15:20:07Z

pkg/metrics/statsd/golang/metrics.go

@@ -34,11 +35,11 @@ func NewMetrics(log *logrus.Entry, m metrics.Interface) (Runnable, error) {
 		r:   prometheus.NewRegistry(),
 	}

-	if err := g.r.Register(prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{})); err != nil {
+	if err := g.r.Register(collectors.NewProcessCollector(collectors.ProcessCollectorOpts{})); err != nil {


github.com/prometheus/client_golang/prometheus got updated and prometheus.NewProcessCollector while still exists - it is depricated. Our golangci-lint job complains about it.

mjudeikis · 2021-09-09T09:24:16Z

overall looks good! :)

m1kola · 2021-09-09T09:28:27Z

Previous e2e run failed, but it looked like a flake. Waiting for another e2e run, but I think this is ready for review now.

m1kola · 2021-09-09T11:20:50Z

I created a separate PR on top of this one to set min TLS version to 1.2 for storage accoutns to reflect what installer is doing: #1730.

We need to merge #1730 after this PR.

ross-bryan

Looks good thanks for detailed explanation with links etc.. in comments. Much easier to understand with that information available.

ross-bryan · 2021-09-09T12:21:21Z

pkg/operator/deploy/staticresources/aro.openshift.io_clusters.yaml

@@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
  annotations:
-    controller-gen.kubebuilder.io/version: v0.5.0
+    controller-gen.kubebuilder.io/version: v0.6.2


sorry for my ignorance, where does this version come from?

@bryanro92 Good question. I should've explained it too as it is not obvious.

We generate this yaml using controller-gen which is part of sigs.k8s.io/controller-tools. It got updated from v0.5.0 to v0.6.2 and as a result on make generate this yaml got updated too.

You can see version change of sigs.k8s.io/controller-tools in go.mod.

pkg/util/version/const.go

jewzaam · 2021-09-09T17:01:54Z

/azp run e2e

azure-pipelines · 2021-09-09T17:02:05Z

Azure Pipelines successfully started running 1 pipeline(s).

m1kola force-pushed the installer-release-4.8-azure branch 3 times, most recently from 8d574c4 to 06a9ee2 Compare September 7, 2021 09:42

m1kola changed the title Release 4.8.* Release 4.8.10 Sep 7, 2021

m1kola force-pushed the installer-release-4.8-azure branch 5 times, most recently from 4dc6416 to 4375e77 Compare September 8, 2021 13:00

m1kola commented Sep 8, 2021

View reviewed changes

Mikalai Radchuk added 2 commits September 8, 2021 15:53

OCP 4.8 release preparation

f062e43

Updates the codebase to allow vendoring of OCP 4.8

make vendor

b873abf

m1kola force-pushed the installer-release-4.8-azure branch from 4375e77 to b4b4fd2 Compare September 8, 2021 14:58

m1kola commented Sep 8, 2021

View reviewed changes

m1kola marked this pull request as ready for review September 9, 2021 09:27

m1kola requested review from jewzaam and mjudeikis as code owners September 9, 2021 09:27

m1kola requested a review from rolandmkunkel September 9, 2021 09:27

mjudeikis previously approved these changes Sep 9, 2021

View reviewed changes

m1kola requested a review from bennerv September 9, 2021 11:11

m1kola mentioned this pull request Sep 9, 2021

Sets min TLS version on storage account to TLS 1.2 #1730

Closed

ross-bryan previously approved these changes Sep 9, 2021

View reviewed changes

jewzaam requested changes Sep 9, 2021

View reviewed changes

pkg/util/version/const.go Show resolved Hide resolved

Mikalai Radchuk added 3 commits September 9, 2021 15:00

Sets OCP 4.8.10 as an install stream

f4eb760

make generate

e3cc89f

Sets min TLS version on storage account to TLS 1.2

319fa70

m1kola dismissed ross-bryan’s stale review via 319fa70 September 9, 2021 14:00

m1kola dismissed mjudeikis’s stale review via 319fa70 September 9, 2021 14:00

m1kola force-pushed the installer-release-4.8-azure branch from b4b4fd2 to 319fa70 Compare September 9, 2021 14:00

m1kola requested a review from jewzaam September 9, 2021 14:01

jewzaam approved these changes Sep 9, 2021

View reviewed changes

jewzaam merged commit c2def8a into Azure:master Sep 9, 2021

m1kola deleted the installer-release-4.8-azure branch September 10, 2021 10:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release 4.8.10 #1703

Release 4.8.10 #1703

m1kola commented Aug 26, 2021

m1kola left a comment

m1kola Sep 8, 2021

m1kola Sep 8, 2021

m1kola Sep 8, 2021

m1kola Sep 8, 2021

m1kola Sep 8, 2021

m1kola Sep 8, 2021

m1kola Sep 8, 2021

m1kola Sep 8, 2021

mjudeikis commented Sep 9, 2021

m1kola commented Sep 9, 2021

m1kola commented Sep 9, 2021

ross-bryan left a comment

ross-bryan Sep 9, 2021

m1kola Sep 9, 2021

jewzaam commented Sep 9, 2021

azure-pipelines bot commented Sep 9, 2021

Release 4.8.10 #1703

Release 4.8.10 #1703

Conversation

m1kola commented Aug 26, 2021

Which issue this PR addresses:

What this PR does / why we need it:

Test plan for issue:

Is there any documentation that needs to be updated for this PR?

m1kola left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjudeikis commented Sep 9, 2021

m1kola commented Sep 9, 2021

m1kola commented Sep 9, 2021

ross-bryan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jewzaam commented Sep 9, 2021

azure-pipelines bot commented Sep 9, 2021