Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Update Kubernetes deps to use release-1.8 branches #47

Merged
merged 6 commits into from
Oct 22, 2017
Merged

Update Kubernetes deps to use release-1.8 branches #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f336cac
Update Kubernetes deps to use release-1.8 branches
Oct 19, 2017
e0d659f
Use generate-internal-groups.sh script
Oct 19, 2017
1fe4629
Regenerate files
Oct 19, 2017
d37bc4e
Update import paths
Oct 19, 2017
70d8400
Fix up pkg/apiserver for changes between master and release-1.8
Oct 19, 2017
c6c352a
Fix conversion generation without k8s.io/kubernetes present
Oct 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
56 changes: 43 additions & 13 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 7 additions & 11 deletions Gopkg.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,22 @@ required = [
"k8s.io/code-generator/cmd/lister-gen",
]

[[constraint]]
name = "github.com/Sirupsen/logrus"
revision = "v0.6.2-10-g51fe59a"
Copy link
Member

Choose a reason for hiding this comment

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

❓ I guess this is because we've switched to glog in navigator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep - this was just a constraint since we removed use of logrus (the package itself wasn't actually depended upon since before this PR)


[[constraint]]
name = "k8s.io/client-go"
branch = "master"
branch = "release-5.0"
Copy link
Member

Choose a reason for hiding this comment

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

❓ Is there any advantage to using a version directive here? Will the release branch be updated with patch releases etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the release branches are updated. We'll need to run dep ensure -update periodically to re-vendor those changes.


[[constraint]]
name = "k8s.io/apimachinery"
branch = "master"
branch = "release-1.8"

[[constraint]]
name = "k8s.io/api"
branch = "master"
branch = "release-1.8"

[[constraint]]
name = "k8s.io/apiserver"
branch = "master"
branch = "release-1.8"

[[override]]
name = "google.golang.org/grpc"
version = "=1.2.1"
[[constraint]]
name = "k8s.io/code-generator"
branch = "release-1.8"
42 changes: 3 additions & 39 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ generate: .generate_files

verify: .hack_verify go_verify

.hack_verify: .generate_exes
.hack_verify:
@echo Running repo-infra verify scripts
@echo Running href checker:
@${HACK_DIR}/verify-links.sh
Expand Down Expand Up @@ -94,43 +94,7 @@ go_fmt:

# This section contains the code generation stuff
#################################################
.generate_exes: \
$(BINDIR)/defaulter-gen \
$(BINDIR)/deepcopy-gen \
$(BINDIR)/conversion-gen \
$(BINDIR)/client-gen \
$(BINDIR)/lister-gen \
$(BINDIR)/informer-gen
touch $@
Copy link
Member

Choose a reason for hiding this comment

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

❓ So we no longer have to compile these binaries? Where do they get compiled now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They now get compiled as part of the new script in code-generator. See: kubernetes/kubernetes#52186


$(BINDIR)/%:
go build -o $@ ./vendor/k8s.io/code-generator/cmd/$*

# Regenerate all files if the gen exes changed or any "types.go" files changed
.generate_files: .generate_exes $(TYPES_FILES)
# Generate defaults
$(BINDIR)/defaulter-gen \
--v 1 --logtostderr \
--go-header-file "$(HACK_DIR)/boilerplate.go.txt" \
--input-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator" \
--input-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator/v1alpha1" \
--extra-peer-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator" \
--extra-peer-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator/v1alpha1" \
--output-file-base "zz_generated.defaults"
# Generate deep copies
$(BINDIR)/deepcopy-gen \
--v 1 --logtostderr \
--go-header-file "$(HACK_DIR)/boilerplate.go.txt" \
--input-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator" \
--input-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator/v1alpha1" \
--bounding-dirs "github.com/openshift/open-service-broker-sdk" \
--output-file-base zz_generated.deepcopy
# Generate conversions
$(BINDIR)/conversion-gen \
--v 1 --logtostderr \
--go-header-file "$(HACK_DIR)/boilerplate.go.txt" \
--input-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator" \
--input-dirs "$(NAVIGATOR_PKG)/pkg/apis/navigator/v1alpha1" \
--output-file-base zz_generated.conversion
# Regenerate all files if any "types.go" files changed
.generate_files: $(TYPES_FILES)
# generate all pkg/client contents
$(HACK_DIR)/update-client-gen.sh
6 changes: 3 additions & 3 deletions cmd/apiserver/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import (

"github.com/jetstack-experimental/navigator/pkg/apis/navigator/v1alpha1"
"github.com/jetstack-experimental/navigator/pkg/apiserver"
clientset "github.com/jetstack-experimental/navigator/pkg/client/clientset_generated/internalclientset"
informers "github.com/jetstack-experimental/navigator/pkg/client/informers_generated/internalversion"
clientset "github.com/jetstack-experimental/navigator/pkg/client/clientset/internalversion"
informers "github.com/jetstack-experimental/navigator/pkg/client/informers/internalversion"
Copy link
Member

Choose a reason for hiding this comment

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

❓ So how do we decide whether to use the internalversion or the versioned APIs? I guess I can see why the API server its self should use the internal versions, but why shouldn't the controller also use the internal APIs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Internal types should only be used within the API machinery for the apiserver, or if for some reason you need to convert between two external versions. That said, our controller should be built against one and only one version of the API, so there should basically never be a requirement to convert locally.

The controller shouldn't use the internal types for a number of reasons:

  • These types can change, breaking compatibility.
  • If we were to use the internal types, we'd then need to convert back into a versioned type in order to submit it back to the API server. We'd also need to convert to the internal version clientside too.
  • I'd question what we have to gain from trying to do this
  • It's also against all best practices for this sort of stuff, and would be a departure from how it is done within the k8s codebase. I can't recall an issue or justification for this decision, but I'm certain there is one with good reason.

There are a number of other potential complexities in doing this, and @dippynark and I did discuss it recently. I've managed to forget most of our thoughts on it though now! I will update here if they come back to me 😄

)

const defaultEtcdPathPrefix = "/registry/navigator.jetstack.io"
Expand Down Expand Up @@ -104,7 +104,7 @@ func (o NavigatorServerOptions) Config() (*apiserver.Config, error) {
return nil, fmt.Errorf("error creating self-signed certificates: %v", err)
}

serverConfig := genericapiserver.NewConfig(apiserver.Codecs)
serverConfig := genericapiserver.NewRecommendedConfig(apiserver.Codecs)
if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/controller/app/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"k8s.io/client-go/tools/record"

"github.com/jetstack-experimental/navigator/cmd/controller/app/options"
clientset "github.com/jetstack-experimental/navigator/pkg/client/clientset_generated/clientset"
intscheme "github.com/jetstack-experimental/navigator/pkg/client/clientset_generated/clientset/scheme"
clientset "github.com/jetstack-experimental/navigator/pkg/client/clientset/versioned"
intscheme "github.com/jetstack-experimental/navigator/pkg/client/clientset/versioned/scheme"
"github.com/jetstack-experimental/navigator/pkg/controllers"
"github.com/jetstack-experimental/navigator/pkg/kube"
)
Expand Down
Loading