Skip to content

Commit

Permalink
Remove reposPerNamespace feature flag. (#1799)
Browse files Browse the repository at this point in the history
* Remove reposPerNamespace feature flag.

Leaves the flag available for the apprepository controller binary though
it does nothing (and can be removed later when people have upgraded).

* Remove comment
  • Loading branch information
absoludity authored Jun 17, 2020
1 parent 94fe483 commit 221c4b7
Show file tree
Hide file tree
Showing 18 changed files with 43 additions and 146 deletions.
2 changes: 0 additions & 2 deletions chart/kubeapps/templates/apprepository-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ spec:
{{- if .Values.apprepository.crontab }}
- --crontab={{ .Values.apprepository.crontab }}
{{- end }}
{{- if .Values.featureFlags.reposPerNamespace }}
- --repos-per-namespace
{{- end }}
{{- if .Values.apprepository.resources }}
resources: {{- toYaml .Values.apprepository.resources | nindent 12 }}
{{- end }}
1 change: 0 additions & 1 deletion chart/kubeapps/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,6 @@ authProxy:
## Feature flags
## These are used to switch on in development features or new features which are ready to be released.
featureFlags:
reposPerNamespace: true
invalidateCache: true
operators: false
# additionalClusters is a WIP feature for multi-cluster support.
Expand Down
4 changes: 3 additions & 1 deletion cmd/apprepository-controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ const (
// existing.
ErrResourceExists = "ErrResourceExists"

LabelRepoName = "apprepositories.kubeapps.com/repo-name"
// LabelRepoName is the label used to identify the repository name.
LabelRepoName = "apprepositories.kubeapps.com/repo-name"
// LabelRepoNamespace is the label used to identify the repository namespace.
LabelRepoNamespace = "apprepositories.kubeapps.com/repo-namespace"

// MessageResourceExists is the message used for Events when a resource
Expand Down
11 changes: 3 additions & 8 deletions cmd/apprepository-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
kubeinformers "k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/clientcmd" // Uncomment the following line to load the gcp plugin (only required to authenticate against GKE clusters).

// _ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
log "github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -69,13 +70,7 @@ func main() {

// We're interested in being informed about cronjobs in kubeapps namespace only, currently.
kubeInformerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, 0, kubeinformers.WithNamespace(namespace))
// Depending on the flag, we may be interested in AppRepository resources across the cluster.
var apprepoInformerFactory informers.SharedInformerFactory
if reposPerNamespace {
apprepoInformerFactory = informers.NewSharedInformerFactory(apprepoClient, 0)
} else {
apprepoInformerFactory = informers.NewFilteredSharedInformerFactory(apprepoClient, 0, namespace, nil)
}
apprepoInformerFactory := informers.NewSharedInformerFactory(apprepoClient, 0)

controller := NewController(kubeClient, apprepoClient, kubeInformerFactory, apprepoInformerFactory, namespace)

Expand All @@ -93,7 +88,7 @@ func init() {
flag.StringVar(&repoSyncImage, "repo-sync-image", "quay.io/helmpack/chart-repo:latest", "container repo/image to use in CronJobs")
flag.StringVar(&repoSyncCommand, "repo-sync-cmd", "/chart-repo", "command used to sync/delete repos for repo-sync-image")
flag.StringVar(&namespace, "namespace", "kubeapps", "Namespace to discover AppRepository resources")
flag.BoolVar(&reposPerNamespace, "repos-per-namespace", false, "Enables syncing app repositories across all namespaces.")
flag.BoolVar(&reposPerNamespace, "repos-per-namespace", true, "UNUSED: This flag will be removed in a future release.")
flag.StringVar(&dbType, "database-type", "mongodb", "Database type. Allowed values: mongodb, postgresql")
flag.StringVar(&dbURL, "database-url", "localhost", "Database URL")
flag.StringVar(&dbUser, "database-user", "root", "Database user")
Expand Down
1 change: 0 additions & 1 deletion dashboard/public/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"oauthLoginURI": "/oauth2/start",
"oauthLogoutURI": "/oauth2/sign_out",
"featureFlags": {
"reposPerNamespace": true,
"operators": true
}
}
29 changes: 1 addition & 28 deletions dashboard/src/actions/repos.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,35 +98,8 @@ actionTestCases.forEach(tc => {
describe("deleteRepo", () => {
context("dispatches requestRepos and receivedRepos after deletion if no error", async () => {
const currentNamespace = "current-namespace";
it("dispatches requestRepos with kubeapps namespace when reposPerNamespace is not set", async () => {
it("dispatches requestRepos with current namespace", async () => {
const storeWithFlag: any = mockStore({
config: {
namespace: kubeappsNamespace,
featureFlags: { reposPerNamespace: false },
},
namespace: { current: currentNamespace },
});
const expectedActions = [
{
type: getType(repoActions.requestRepos),
payload: kubeappsNamespace,
},
{
type: getType(repoActions.receiveRepos),
payload: { foo: "bar" },
},
];

await storeWithFlag.dispatch(repoActions.deleteRepo("foo", "my-namespace"));
expect(storeWithFlag.getActions()).toEqual(expectedActions);
});

it("dispatches requestRepos with current namespace when reposPerNamespace is set", async () => {
const storeWithFlag: any = mockStore({
config: {
namespace: kubeappsNamespace,
featureFlags: { reposPerNamespace: true },
},
namespace: { current: currentNamespace },
});
const expectedActions = [
Expand Down
12 changes: 4 additions & 8 deletions dashboard/src/actions/repos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import { AppRepository } from "../shared/AppRepository";
import Chart from "../shared/Chart";
import { definedNamespaces } from "../shared/Namespace";
import Secret from "../shared/Secret";
import { errorChart } from "./charts";

import {
IAppRepository,
IAppRepositoryKey,
ISecret,
IStoreState,
NotFoundError,
NotFoundError
} from "../shared/types";
import { errorChart } from "./charts";


export const addRepo = createAction("ADD_REPO");
export const addedRepo = createAction("ADDED_REPO", resolve => {
Expand Down Expand Up @@ -115,14 +115,10 @@ export const deleteRepo = (
return async (dispatch, getState) => {
const {
namespace: { current },
config: { namespace: kubeappsNamespace, featureFlags },
} = getState();
try {
await AppRepository.delete(name, namespace);
const fetchFromNamespace: string = featureFlags.reposPerNamespace
? current
: kubeappsNamespace;
dispatch(fetchRepos(fetchFromNamespace));
dispatch(fetchRepos(current));
return true;
} catch (e) {
dispatch(errorRepos(e, "delete"));
Expand Down
22 changes: 4 additions & 18 deletions dashboard/src/components/Header/Header.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { shallow } from "enzyme";
import * as React from "react";

import { INamespaceState } from "../../reducers/namespace";
import Header from "./Header";


const defaultProps = {
authenticated: true,
fetchNamespaces: jest.fn(),
Expand Down Expand Up @@ -35,23 +35,9 @@ it("renders the header links and titles", () => {
});

describe("settings", () => {
it("renders settings without reposPerNamespace", () => {
const wrapper = shallow(<Header {...defaultProps} />);
const settingsbar = wrapper.find(".header__nav__submenu").first();
const items = settingsbar.find("NavLink").map(p => p.props());
const expectedItems = [
{ children: "App Repositories", to: "/config/repos" },
{ children: "Service Brokers", to: "/config/brokers" },
];
items.forEach((item, index) => {
expect(item.children).toBe(expectedItems[index].children);
expect(item.to).toBe(expectedItems[index].to);
});
});

it("renders settings with reposPerNamespace", () => {
it("renders settings", () => {
const wrapper = shallow(
<Header {...defaultProps} featureFlags={{ reposPerNamespace: true, operators: false }} />,
<Header {...defaultProps} featureFlags={{ operators: false }} />,
);
const settingsbar = wrapper.find(".header__nav__submenu").first();
const items = settingsbar.find("NavLink").map(p => p.props());
Expand All @@ -67,7 +53,7 @@ describe("settings", () => {

it("renders operators link", () => {
const wrapper = shallow(
<Header {...defaultProps} featureFlags={{ reposPerNamespace: true, operators: true }} />,
<Header {...defaultProps} featureFlags={{ operators: true }} />,
);
const settingsbar = wrapper.find(".header__nav__submenu").first();
const items = settingsbar.find("NavLink").map(p => p.props());
Expand Down
18 changes: 8 additions & 10 deletions dashboard/src/components/Header/Header.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import * as React from "react";
// Icons
import { LogOut, Settings } from "react-feather";
import { NavLink } from "react-router-dom";
import "react-select/dist/react-select.css";
import logo from "../../logo.svg";

import { INamespaceState } from "../../reducers/namespace";
import { definedNamespaces } from "../../shared/Namespace";
import "./Header.css";
import HeaderLink, { IHeaderLinkProps } from "./HeaderLink";
import NamespaceSelector from "./NamespaceSelector";

// Icons
import { LogOut, Settings } from "react-feather";

import "react-select/dist/react-select.css";
import "./Header.css";


interface IHeaderProps {
authenticated: boolean;
Expand All @@ -24,7 +24,7 @@ interface IHeaderProps {
setNamespace: (ns: string) => void;
createNamespace: (ns: string) => Promise<boolean>;
getNamespace: (ns: string) => void;
featureFlags: { reposPerNamespace: boolean; operators: boolean };
featureFlags: { operators: boolean };
}

interface IHeaderState {
Expand All @@ -34,7 +34,7 @@ interface IHeaderState {

class Header extends React.Component<IHeaderProps, IHeaderState> {
public static defaultProps = {
featureFlags: { reposPerNamespace: false, operators: false },
featureFlags: { operators: false },
};

// Defines the route
Expand Down Expand Up @@ -89,9 +89,7 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
this.state.configOpen ? "header__nav__submenu-open" : ""
}`;

const reposPath = this.props.featureFlags.reposPerNamespace
? `/config/ns/${namespace.current}/repos`
: "/config/repos";
const reposPath = `/config/ns/${namespace.current}/repos`;
return (
<section className="gradient-135-brand type-color-reverse type-color-reverse-anchor-reset">
<div className="container">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const defaultState = {
auth: defaultAuthState,
router: { location: emptyLocation },
config: {
featureFlags: { reposPerNamespace: true },
featureFlags: { operators: true },
},
};

Expand All @@ -43,15 +43,15 @@ describe("HeaderContainer props", () => {
const store = mockStore({
...defaultState,
config: {
featureFlags: { reposPerNamespace: true },
featureFlags: { operators: true },
},
});

const wrapper = shallow(<Header store={store} />);

const form = wrapper.find("Header");
expect(form).toHaveProp({
featureFlags: { reposPerNamespace: true },
featureFlags: { operators: true },
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const makeStore = (
namespace: "",
appVersion: "",
oauthLogoutURI: "",
featureFlags: { reposPerNamespace: false, operators: false },
featureFlags: { operators: false },
};
return mockStore({ auth, config });
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ import { shallow } from "enzyme";
import * as React from "react";
import configureMockStore from "redux-mock-store";
import thunk from "redux-thunk";

import RepoListContainer from ".";
import { definedNamespaces } from "../../shared/Namespace";


const mockStore = configureMockStore([thunk]);
const currentNamespace = "current-namespace";
const kubeappsNamespace = "kubeapps-namespace";

const defaultState = {
config: {
featureFlags: { reposPerNamespace: false },
namespace: kubeappsNamespace,
},
namespace: { current: currentNamespace },
Expand All @@ -21,24 +20,9 @@ const defaultState = {
};

describe("RepoListContainer props", () => {
it("uses kubeapps namespace when reposPerNamespace false", () => {
const store = mockStore(defaultState);
const wrapper = shallow(<RepoListContainer store={store} />);

const component = wrapper.find("AppRepoList");

expect(component).toHaveProp({
namespace: kubeappsNamespace,
displayReposPerNamespaceMsg: false,
});
});

it("uses the current namespace when reposPerNamespace is true", () => {
it("uses the current namespace", () => {
const store = mockStore({
...defaultState,
config: {
featureFlags: { reposPerNamespace: true },
},
});
const wrapper = shallow(<RepoListContainer store={store} />);

Expand All @@ -50,11 +34,10 @@ describe("RepoListContainer props", () => {
});
});

it("passes _all through as a normal namespace when reposPerNamespaces is true, to be handled by the component", () => {
it("passes _all through as a normal namespace to be handled by the component", () => {
const store = mockStore({
...defaultState,
config: {
featureFlags: { reposPerNamespace: true },
namespace: kubeappsNamespace,
},
namespace: { current: definedNamespaces.all },
Expand Down
11 changes: 4 additions & 7 deletions dashboard/src/containers/RepoListContainer/RepoListContainer.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
import { connect } from "react-redux";
import { Action } from "redux";
import { ThunkDispatch } from "redux-thunk";

import actions from "../../actions";
import AppRepoList from "../../components/Config/AppRepoList";
import { definedNamespaces } from "../../shared/Namespace";
import { IAppRepositoryKey, IStoreState } from "../../shared/types";


function mapStateToProps({ config, namespace, repos }: IStoreState) {
let repoNamespace = config.namespace;
const repoNamespace = namespace.current;
let displayReposPerNamespaceMsg = false;
if (config.featureFlags.reposPerNamespace) {
repoNamespace = namespace.current;
if (repoNamespace !== definedNamespaces.all) {
displayReposPerNamespaceMsg = true;
}
if (repoNamespace !== definedNamespaces.all) {
displayReposPerNamespaceMsg = true;
}
return {
errors: repos.errors,
Expand Down
Loading

0 comments on commit 221c4b7

Please sign in to comment.