Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Bump Kubernetes version 1.14 #2605

Merged
merged 1 commit into from
Apr 17, 2019

Conversation

jberkhahn
Copy link
Contributor

@jberkhahn jberkhahn commented Apr 10, 2019

This bumps the versions of our kube and client-go dependencies. See comments below for specifics.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 10, 2019
@jberkhahn
Copy link
Contributor Author

/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 10, 2019
@jberkhahn jberkhahn force-pushed the bump_kube_1.14 branch 3 times, most recently from de8c487 to ad0222c Compare April 11, 2019 00:02
@jberkhahn
Copy link
Contributor Author

/test pull-service-catalog-integration

@@ -86,7 +86,7 @@ items:
# TODO: do not grant global access, limit to particular secrets referenced from servicebindings
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get","create","update","delete"]
verbs: ["get","create","update","delete", "list", "watch"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client-go is listing secrets when the controller starts up now, i couldn't find a way to stop it from doing this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's weird. probably some upstream change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we never have fixed that comment.

@@ -192,7 +191,7 @@ func buildAdmission(c *genericapiserver.RecommendedConfig, s *ServiceCatalogServ
pluginNames := enabledPluginNames(s.AdmissionOptions)
klog.Infof("Admission control plugin names: %v", pluginNames)

genericInitializer := initializer.New(kubeClient, kubeSharedInformers, c.Authorization.Authorizer, api.Scheme)
genericInitializer := initializer.New(kubeClient, kubeSharedInformers, c.Authorization.Authorizer)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was deprecated on kube's end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's super. it's taken a long.

@@ -247,14 +248,20 @@ func Run(controllerManagerOptions *options.ControllerManagerServer) error {
return err
}

klog.V(5).Infof("Using namespace %v for leader election lock", controllerManagerOptions.LeaderElectionNamespace)
// create config for interacting with coordination.k8s.io group
coordinationClient, err := v1coordination.NewForConfig(k8sKubeconfig)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really sure what this api group is for, it might just be they're trying to move things out of the default group. They had the initializer for it so I'm not too worred.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like leader election leases.

I don't think we have a test for that.

@@ -120,7 +120,7 @@ func (s *ControllerManagerServer) AddFlags(fs *pflag.FlagSet) {
fs.DurationVar(&s.ReconciliationRetryDuration, "reconciliation-retry-duration", s.ReconciliationRetryDuration, "The maximum amount of time to retry reconciliations on a resource before failing")
fs.DurationVar(&s.OperationPollingMaximumBackoffDuration, "operation-polling-maximum-backoff-duration", s.OperationPollingMaximumBackoffDuration, "The maximum amount of time to back-off while polling an OSB API operation")
s.SecureServingOptions.AddFlags(fs)
utilfeature.DefaultFeatureGate.AddFlag(fs)
utilfeature.DefaultMutableFeatureGate.AddFlag(fs)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same thing, the name just changed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are they per instance somehow now? we had parallel threading issues with featuregates in the past.

func mergeGroupEncodingConfigs(defaultResourceEncoding *serverstorage.DefaultResourceEncodingConfig, storageEncodingOverrides map[string]schema.GroupVersion) *serverstorage.DefaultResourceEncodingConfig {
resourceEncodingConfig := defaultResourceEncoding
for group, storageEncodingVersion := range storageEncodingOverrides {
resourceEncodingConfig.SetVersionEncoding(group, storageEncodingVersion, schema.GroupVersion{Group: group, Version: runtime.APIVersionInternal})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetVersionEncoding was straight up removed. I don't see an equivalent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that should be fine.

@@ -29,11 +29,7 @@ import (
type Interface interface {
Discovery() discovery.DiscoveryInterface
ServicecatalogV1beta1() servicecatalogv1beta1.ServicecatalogV1beta1Interface
// Deprecated: please explicitly pick a version if possible.
Servicecatalog() servicecatalogv1beta1.ServicecatalogV1beta1Interface
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was finally ripped out, so I had to move everything over to using the specific version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, the generated code had a warning. I'd never have seen it until now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super curious about the history of this, but not enough to spend more than 3 minutes on it.

@@ -839,8 +839,8 @@ func TestReconcileServiceInstanceAppliesDefaultProvisioningParams(t *testing.T)
}
}

func TestReconcileServiceInstanceRespectsServicePlanDefaultsFeatureGate(t *testing.T) {
err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.ServicePlanDefaults))
func TestReconcileServiceInstanceRespectsServicePlanDefaultsMutableFeatureGate(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the name here as well

@@ -21,7 +21,7 @@ import (
"strings"

utiltemplate "github.com/kubernetes-incubator/service-catalog/pkg/kubernetes/pkg/util/template"
"k8s.io/apiserver/pkg/util/flag"
"k8s.io/component-base/cli/flag"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the same package, it just got moved into its own repo

}
return labels.Set(binding.ObjectMeta.Labels), toSelectableFields(binding), binding.Initializers != nil, nil
return labels.Set(binding.ObjectMeta.Labels), toSelectableFields(binding), nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was also just removed

@@ -66,7 +66,7 @@ func convertToSARExtra(extra map[string][]string) map[string]authorizationapi.Ex
return ret
}

func (s *sarcheck) Admit(a admission.Attributes) error {
func (s *sarcheck) Admit(a admission.Attributes, o admission.ObjectInterfaces) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was added to the interface, not really sure what we're supposed to do with it...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something to do for CRDs kubernetes/kubernetes#74154
feel free to browse through that.

@@ -240,7 +240,7 @@ func TestAdmissionBroker(t *testing.T) {
}
kubeInformerFactory.Start(wait.NeverStop)

err = handler.(admission.MutationInterface).Admit(admission.NewAttributesRecord(tc.broker, nil, servicecatalog.Kind("ClusterServiceBroker").WithVersion("version"), tc.broker.Namespace, tc.broker.Name, servicecatalog.Resource("clusterservicebrokers").WithVersion("version"), "", admission.Create, false, tc.userInfo))
err = handler.(admission.MutationInterface).Admit(admission.NewAttributesRecord(tc.broker, nil, servicecatalog.Kind("ClusterServiceBroker").WithVersion("version"), tc.broker.Namespace, tc.broker.Name, servicecatalog.Resource("clusterservicebrokers").WithVersion("version"), "", admission.Create, false, tc.userInfo), nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left it as nil unless and until we figure out what to do with it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be fine.

@@ -110,7 +110,7 @@ func TestEtcdHealthCheckerSuccess(t *testing.T) {
}

func testGroupVersion(client servicecatalogclient.Interface) error {
gv := client.Servicecatalog().RESTClient().APIVersion()
gv := client.ServicecatalogV1beta1().RESTClient().APIVersion()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-versioned way was deprecated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, totally gone?

@@ -76,7 +76,7 @@ func withConfigGetFreshApiserverAndClient(

var etcdOptions *server.EtcdOptions
etcdOptions = server.NewEtcdOptions()
etcdOptions.StorageConfig.ServerList = serverConfig.etcdServerList
etcdOptions.StorageConfig.Transport.ServerList = serverConfig.etcdServerList
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing, it just got moved into a sub-struct on the StorageConfig

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find that a bizarre name. it's not a transport or anything like a transport.

que sera

@jberkhahn
Copy link
Contributor Author

@MHBauer @jboyd01 this is ready for review

@MHBauer
Copy link
Contributor

MHBauer commented Apr 15, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 15, 2019
@MHBauer
Copy link
Contributor

MHBauer commented Apr 16, 2019

why is this hold?

@jberkhahn jberkhahn removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 16, 2019
@jberkhahn
Copy link
Contributor Author

@piotrmiskiewicz took a look at this and gave it the thumbs up. it will require some changes to their CRD branch, but they're cool with that

@jboyd01
Copy link
Contributor

jboyd01 commented Apr 17, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jboyd01

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 17, 2019
@k8s-ci-robot k8s-ci-robot merged commit 7fe8aaf into kubernetes-retired:master Apr 17, 2019
viviyww pushed a commit to viviyww/service-catalog that referenced this pull request May 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants